Return the full rejection message and errors in flash errors (#13221) (#13237)

* Return the full rejection message and errors in flash errors (#13221)

Signed-off-by: Andrew Thornton <art27@cantab.net>
* Update routers/repo/pull.go

Co-authored-by: John Olheiser <john.olheiser@gmail.com>
Co-authored-by: John Olheiser <john.olheiser@gmail.com>

diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 16d3874e1bec8a94aec57c2ac856f92bde86df7a..18c621db67dfad066695d1b131e490065b2e18fc 100644 (file)
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -870,9 +870,11 @@ editor.file_already_exists = A file named '%s' already exists in this repository
 editor.commit_empty_file_header = Commit an empty file
 editor.commit_empty_file_text = The file you're about to commit is empty. Proceed?
 editor.no_changes_to_show = There are no changes to show.
-editor.fail_to_update_file = Failed to update/create file '%s' with error: %v
+editor.fail_to_update_file = Failed to update/create file '%s'.
+editor.fail_to_update_file_summary = Error Message:
 editor.push_rejected_no_message = The change was rejected by the server without a message. Please check githooks.
-editor.push_rejected = The change was rejected by the server with the following message:<br>%s<br> Please check githooks.
+editor.push_rejected = The change was rejected by the server. Please check githooks.
+editor.push_rejected_summary = Full Rejection Message:
 editor.add_subdir = Add a directory…
 editor.unable_to_upload_files = Failed to upload files to '%s' with error: %v
 editor.upload_file_is_locked = File '%s' is locked by %s.
@@ -1259,11 +1261,15 @@ pulls.rebase_merge_commit_pull_request = Rebase and Merge (--no-ff)
 pulls.squash_merge_pull_request = Squash and Merge
 pulls.require_signed_wont_sign = The branch requires signed commits but this merge will not be signed
 pulls.invalid_merge_option = You cannot use this merge option for this pull request.
-pulls.merge_conflict = Merge Failed: There was a conflict whilst merging: %[1]s<br>%[2]s<br>Hint: Try a different strategy
-pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %[1]s<br>%[2]s<br>%[3]s<br>Hint:Try a different strategy
+pulls.merge_conflict = Merge Failed: There was a conflict whilst merging. Hint: Try a different strategy
+pulls.merge_conflict_summary = Error Message
+pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %[1]s. Hint: Try a different strategy
+pulls.rebase_conflict_summary = Error Message
+; </summary><code>%[2]s<br>%[3]s</code></details>
 pulls.unrelated_histories = Merge Failed: The merge head and base do not share a common history. Hint: Try a different strategy
 pulls.merge_out_of_date = Merge Failed: Whilst generating the merge, the base was updated. Hint: Try again.
-pulls.push_rejected = Merge Failed: The push was rejected with the following message:<br>%s<br>Review the githooks for this repository
+pulls.push_rejected = Merge Failed: The push was rejected. Review the githooks for this repository. 
+pulls.push_rejected_summary = Full Rejection Message
 pulls.push_rejected_no_message = Merge Failed: The push was rejected but there was no remote message.<br>Review the githooks for this repository
 pulls.open_unmerged_pull_exists = `You cannot perform a reopen operation because there is a pending pull request (#%d) with identical properties.`
 pulls.status_checking = Some checks are pending

diff --git a/routers/repo/branch.go b/routers/repo/branch.go
index 0ca77cbf6f2de0366a99d3857bc2b37c64e671ed..cd18f667772cbf612ad535c999162e387c5190bd 100644 (file)
--- a/routers/repo/branch.go
+++ b/routers/repo/branch.go
@@ -355,7 +355,16 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) {
                        if len(e.Message) == 0 {
                                ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected_no_message"))
                        } else {
-                               ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(e.Message)))
+                               flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+                                       "Message": ctx.Tr("repo.editor.push_rejected"),
+                                       "Summary": ctx.Tr("repo.editor.push_rejected_summary"),
+                                       "Details": utils.SanitizeFlashErrorString(e.Message),
+                               })
+                               if err != nil {
+                                       ctx.ServerError("UpdatePullRequest.HTMLString", err)
+                                       return
+                               }
+                               ctx.Flash.Error(flashError)
                        }
                        ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
                        return

diff --git a/routers/repo/editor.go b/routers/repo/editor.go
index a08c7193f87022f3be7a99fee06dd10773cebf62..1ee557a4fdf1e9a66d6ce21216f1e9ff52b52cb0 100644 (file)
--- a/routers/repo/editor.go
+++ b/routers/repo/editor.go
@@ -293,10 +293,28 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo
                        if len(errPushRej.Message) == 0 {
                                ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplEditFile, &form)
                        } else {
-                               ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplEditFile, &form)
+                               flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+                                       "Message": ctx.Tr("repo.editor.push_rejected"),
+                                       "Summary": ctx.Tr("repo.editor.push_rejected_summary"),
+                                       "Details": utils.SanitizeFlashErrorString(errPushRej.Message),
+                               })
+                               if err != nil {
+                                       ctx.ServerError("editFilePost.HTMLString", err)
+                                       return
+                               }
+                               ctx.RenderWithErr(flashError, tplEditFile, &form)
                        }
                } else {
-                       ctx.RenderWithErr(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, utils.SanitizeFlashErrorString(err.Error())), tplEditFile, &form)
+                       flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+                               "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()),
+                       })
+                       if err != nil {
+                               ctx.ServerError("editFilePost.HTMLString", err)
+                               return
+                       }
+                       ctx.RenderWithErr(flashError, tplEditFile, &form)
                }
        }
 
@@ -464,7 +482,16 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) {
                        if len(errPushRej.Message) == 0 {
                                ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplDeleteFile, &form)
                        } else {
-                               ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplDeleteFile, &form)
+                               flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+                                       "Message": ctx.Tr("repo.editor.push_rejected"),
+                                       "Summary": ctx.Tr("repo.editor.push_rejected_summary"),
+                                       "Details": utils.SanitizeFlashErrorString(errPushRej.Message),
+                               })
+                               if err != nil {
+                                       ctx.ServerError("DeleteFilePost.HTMLString", err)
+                                       return
+                               }
+                               ctx.RenderWithErr(flashError, tplDeleteFile, &form)
                        }
                } else {
                        ctx.ServerError("DeleteRepoFile", err)
@@ -656,7 +683,16 @@ func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) {
                        if len(errPushRej.Message) == 0 {
                                ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplUploadFile, &form)
                        } else {
-                               ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplUploadFile, &form)
+                               flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+                                       "Message": ctx.Tr("repo.editor.push_rejected"),
+                                       "Summary": ctx.Tr("repo.editor.push_rejected_summary"),
+                                       "Details": utils.SanitizeFlashErrorString(errPushRej.Message),
+                               })
+                               if err != nil {
+                                       ctx.ServerError("UploadFilePost.HTMLString", err)
+                                       return
+                               }
+                               ctx.RenderWithErr(flashError, tplUploadFile, &form)
                        }
                } else {
                        // os.ErrNotExist - upload file missing in the intervening time?!

diff --git a/routers/repo/pull.go b/routers/repo/pull.go
index 54da7b76b7edff18b7ffd7e5671296e0d7f41208..f5c3aec0f8296d4540f56822d2b800f0efe0b2fc 100644 (file)
--- a/routers/repo/pull.go
+++ b/routers/repo/pull.go
@@ -723,7 +723,16 @@ func UpdatePullRequest(ctx *context.Context) {
        if err = pull_service.Update(issue.PullRequest, ctx.User, message); err != nil {
                if models.IsErrMergeConflicts(err) {
                        conflictError := err.(models.ErrMergeConflicts)
-                       ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
+                       flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+                               "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),
+                       })
+                       if err != nil {
+                               ctx.ServerError("UpdatePullRequest.HTMLString", err)
+                               return
+                       }
+                       ctx.Flash.Error(flashError)
                        ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index))
                        return
                }
@@ -846,12 +855,30 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
                        return
                } else if models.IsErrMergeConflicts(err) {
                        conflictError := err.(models.ErrMergeConflicts)
-                       ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
+                       flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+                               "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),
+                       })
+                       if err != nil {
+                               ctx.ServerError("MergePullRequest.HTMLString", err)
+                               return
+                       }
+                       ctx.Flash.Error(flashError)
                        ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
                        return
                } else if models.IsErrRebaseConflicts(err) {
                        conflictError := err.(models.ErrRebaseConflicts)
-                       ctx.Flash.Error(ctx.Tr("repo.pulls.rebase_conflict", utils.SanitizeFlashErrorString(conflictError.CommitSHA), utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
+                       flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+                               "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),
+                       })
+                       if err != nil {
+                               ctx.ServerError("MergePullRequest.HTMLString", err)
+                               return
+                       }
+                       ctx.Flash.Error(flashError)
                        ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
                        return
                } else if models.IsErrMergeUnrelatedHistories(err) {
@@ -871,7 +898,16 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
                        if len(message) == 0 {
                                ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message"))
                        } else {
-                               ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected", utils.SanitizeFlashErrorString(pushrejErr.Message)))
+                               flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+                                       "Message": ctx.Tr("repo.pulls.push_rejected"),
+                                       "Summary": ctx.Tr("repo.pulls.push_rejected_summary"),
+                                       "Details": utils.SanitizeFlashErrorString(pushrejErr.Message),
+                               })
+                               if err != nil {
+                                       ctx.ServerError("MergePullRequest.HTMLString", err)
+                                       return
+                               }
+                               ctx.Flash.Error(flashError)
                        }
                        ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
                        return
@@ -986,7 +1022,16 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm)
                        if len(message) == 0 {
                                ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message"))
                        } else {
-                               ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected", utils.SanitizeFlashErrorString(pushrejErr.Message)))
+                               flashError, err := ctx.HTMLString(string(tplAlertDetails), map[string]interface{}{
+                                       "Message": ctx.Tr("repo.pulls.push_rejected"),
+                                       "Summary": ctx.Tr("repo.pulls.push_rejected_summary"),
+                                       "Details": utils.SanitizeFlashErrorString(pushrejErr.Message),
+                               })
+                               if err != nil {
+                                       ctx.ServerError("CompareAndPullRequest.HTMLString", err)
+                                       return
+                               }
+                               ctx.Flash.Error(flashError)
                        }
                        ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pullIssue.Index))
                        return

diff --git a/routers/repo/repo.go b/routers/repo/repo.go
index 883d30a5d93b8aa1abadf10a18a4b3a70ef57460..742c952f6e7f667e4e8ea894d4497adb9eed5c16 100644 (file)
--- a/routers/repo/repo.go
+++ b/routers/repo/repo.go
@@ -24,7 +24,8 @@ import (
 )
 
 const (
-       tplCreate base.TplName = "repo/create"
+       tplCreate       base.TplName = "repo/create"
+       tplAlertDetails base.TplName = "base/alert_details"
 )
 
 // MustBeNotEmpty render when a repo is a empty git dir

diff --git a/routers/utils/utils.go b/routers/utils/utils.go
index d8060ff07780db56be4dc548b0d2813d167b9b48..f15bc1e62e4be2f66e4cdfbe4ecfa2158bed91cf 100644 (file)
--- a/routers/utils/utils.go
+++ b/routers/utils/utils.go
@@ -41,12 +41,6 @@ func IsValidSlackChannel(channelName string) bool {
 
 // SanitizeFlashErrorString will sanitize a flash error string
 func SanitizeFlashErrorString(x string) string {
-       runes := []rune(x)
-
-       if len(runes) > 512 {
-               x = "..." + string(runes[len(runes)-512:])
-       }
-
        return strings.ReplaceAll(html.EscapeString(x), "\n", "<br>")
 }
 

diff --git a/templates/base/alert.tmpl b/templates/base/alert.tmpl
index 61b99486e27ae725b0b79a858ecae53fbc5a8fe5..cf886f529cac20e16d64c7c06249b6858ca76b45 100644 (file)
--- a/templates/base/alert.tmpl
+++ b/templates/base/alert.tmpl
@@ -1,15 +1,15 @@
 {{if .Flash.ErrorMsg}}
-       <div class="ui negative message">
+       <div class="ui negative message flash-error">
                <p>{{.Flash.ErrorMsg | Str2html}}</p>
        </div>
 {{end}}
 {{if .Flash.SuccessMsg}}
-       <div class="ui positive message">
+       <div class="ui positive message flash-success">
                <p>{{.Flash.SuccessMsg | Str2html}}</p>
        </div>
 {{end}}
 {{if .Flash.InfoMsg}}
-       <div class="ui info message">
+       <div class="ui info message flash-info">
                <p>{{.Flash.InfoMsg | Str2html}}</p>
        </div>
 {{end}}

diff --git a/templates/base/alert_details.tmpl b/templates/base/alert_details.tmpl
new file mode 100644 (file)
index 0000000..38a2721
--- /dev/null
+++ b/templates/base/alert_details.tmpl
@@ -0,0 +1,7 @@
+{{.Message}}
+<details>
+    <summary>{{.Summary}}</summary>
+    <code>
+        {{.Details | Str2html}}
+    </code>
+</details>

diff --git a/web_src/less/_base.less b/web_src/less/_base.less
index dc4f4c6daa1491ca3328456a580fca1adda740f9..765c0f9c930b132d40eaac8908fdf3c4d627981f 100644 (file)
--- a/web_src/less/_base.less
+++ b/web_src/less/_base.less
@@ -1280,3 +1280,8 @@ table th[data-sortt-desc] {
 .ui.header > .ui.label.compact {
   margin-top: inherit;
 }
+
+.flash-error details code {
+  display: block;
+  text-align: left;
+}