From e4e3df6c66dbeac6ac5bceff8ac4f05dbea30d70 Mon Sep 17 00:00:00 2001
From: Gusted <williamzijl7@hotmail.com>
Date: Tue, 28 Dec 2021 13:28:27 +0000
Subject: [PATCH] Handle invalid issues (#18111)
* Handle invalid issues
- When you hover over a issue reference, and the issue doesn't exist, it
will just hang on the loading animation.
- This patch fixes that by showing them the pop-up with a "Error
occured" message.
* Add I18N
* refactor
* fix comment for lint
* fix unit test for i18n
* fix unit test for i18n
* add comments
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
---
integrations/api_releases_test.go | 2 +-
integrations/repo_branch_test.go | 2 +-
modules/context/api.go | 23 ++++++++++++---------
options/locale/locale_en-US.ini | 8 +++++---
templates/base/head.tmpl | 5 ++++-
web_src/js/components/ContextPopup.vue | 28 ++++++++++++++++++--------
6 files changed, 45 insertions(+), 23 deletions(-)
diff --git a/integrations/api_releases_test.go b/integrations/api_releases_test.go
index b3d9c898e4..815b749110 100644
--- a/integrations/api_releases_test.go
+++ b/integrations/api_releases_test.go
@@ -207,7 +207,7 @@ func TestAPIGetReleaseByTag(t *testing.T) {
var err *api.APIError
DecodeJSON(t, resp, &err)
- assert.EqualValues(t, "Not Found", err.Message)
+ assert.NotEmpty(t, err.Message)
}
func TestAPIDeleteReleaseByTagName(t *testing.T) {
diff --git a/integrations/repo_branch_test.go b/integrations/repo_branch_test.go
index af5c475ea7..aef28515e7 100644
--- a/integrations/repo_branch_test.go
+++ b/integrations/repo_branch_test.go
@@ -141,7 +141,7 @@ func TestCreateBranchInvalidCSRF(t *testing.T) {
resp = session.MakeRequest(t, NewRequest(t, "GET", loc), http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
assert.Equal(t,
- "Bad Request: Invalid CSRF token",
+ "Bad Request: invalid CSRF token",
strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
)
}
diff --git a/modules/context/api.go b/modules/context/api.go
index 574d7c4248..635a54c7ef 100644
--- a/modules/context/api.go
+++ b/modules/context/api.go
@@ -30,6 +30,11 @@ type APIContext struct {
Org *APIOrganization
}
+// Currently, we have the following common fields in error response:
+// * message: the message for end users (it shouldn't be used for error type detection)
+// if we need to indicate some errors, we should introduce some new fields like ErrorCode or ErrorType
+// * url: the swagger document URL
+
// APIError is error format response
// swagger:response error
type APIError struct {
@@ -47,8 +52,8 @@ type APIValidationError struct {
// APIInvalidTopicsError is error format response to invalid topics
// swagger:response invalidTopicsError
type APIInvalidTopicsError struct {
- Topics []string `json:"invalidTopics"`
- Message string `json:"message"`
+ Message string `json:"message"`
+ InvalidTopics []string `json:"invalidTopics"`
}
//APIEmpty is an empty response
@@ -122,9 +127,9 @@ func (ctx *APIContext) InternalServerError(err error) {
})
}
-var (
- apiContextKey interface{} = "default_api_context"
-)
+type apiContextKeyType struct{}
+
+var apiContextKey = apiContextKeyType{}
// WithAPIContext set up api context in request
func WithAPIContext(req *http.Request, ctx *APIContext) *http.Request {
@@ -351,7 +356,7 @@ func ReferencesGitRepo(allowEmpty bool) func(http.Handler) http.Handler {
// NotFound handles 404s for APIContext
// String will replace message, errors will be added to a slice
func (ctx *APIContext) NotFound(objs ...interface{}) {
- var message = "Not Found"
+ var message = ctx.Tr("error.not_found")
var errors []string
for _, obj := range objs {
// Ignore nil
@@ -367,9 +372,9 @@ func (ctx *APIContext) NotFound(objs ...interface{}) {
}
ctx.JSON(http.StatusNotFound, map[string]interface{}{
- "message": message,
- "documentation_url": setting.API.SwaggerURL,
- "errors": errors,
+ "message": message,
+ "url": setting.API.SwaggerURL,
+ "errors": errors,
})
}
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 5248f98069..9164d5ffdc 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -104,10 +104,12 @@ error404 = The page you are trying to reach either <strong>does not exist</stron
never = Never
[error]
-occurred = An error has occurred
-report_message = If you are sure this is a Gitea bug, please search for issue on <a href="https://github.com/go-gitea/gitea/issues">GitHub</a> and open new issue if necessary.
+occurred = An error occurred
+report_message = If you are sure this is a Gitea bug, please search for issues on <a href="https://github.com/go-gitea/gitea/issues" target="_blank">GitHub</a> or open a new issue if necessary.
missing_csrf = Bad Request: no CSRF token present
-invalid_csrf = Bad Request: Invalid CSRF token
+invalid_csrf = Bad Request: invalid CSRF token
+not_found = The target couldn't be found.
+network_error = Network error
[startpage]
app_desc = A painless, self-hosted Git service
diff --git a/templates/base/head.tmpl b/templates/base/head.tmpl
index e9c2d512df..8bc6ae689d 100644
--- a/templates/base/head.tmpl
+++ b/templates/base/head.tmpl
@@ -46,10 +46,13 @@
]).values()),
{{end}}
mermaidMaxSourceCharacters: {{MermaidMaxSourceCharacters}},
+ {{/* this global i18n object should only contain gereral texts. for specalized texts, it should be provied inside the related modules by: (1) API response (2) HTML data-attribute (3) PageData */}}
i18n: {
copy_success: '{{.i18n.Tr "copy_success"}}',
copy_error: '{{.i18n.Tr "copy_error"}}',
- }
+ error_occurred: '{{.i18n.Tr "error.occurred"}}',
+ network_error: '{{.i18n.Tr "error.network_error"}}',
+ },
};
{{/* in case some pages don't render the pageData, we make sure it is an object to prevent null access */}}
window.config.pageData = window.config.pageData || {};
diff --git a/web_src/js/components/ContextPopup.vue b/web_src/js/components/ContextPopup.vue
index efaa7be89e..c002a3d066 100644
--- a/web_src/js/components/ContextPopup.vue
+++ b/web_src/js/components/ContextPopup.vue
@@ -16,13 +16,17 @@
</div>
</div>
</div>
+ <div v-if="!loading && issue === null">
+ <p><small>{{ i18nErrorOccurred }}</small></p>
+ <p>{{ i18nErrorMessage }}</p>
+ </div>
</div>
</template>
<script>
import {SvgIcon} from '../svg.js';
-const {appSubUrl} = window.config;
+const {appSubUrl, i18n} = window.config;
// NOTE: see models/issue_label.go for similar implementation
const srgbToLinear = (color) => {
@@ -49,7 +53,9 @@ export default {
data: () => ({
loading: false,
- issue: null
+ issue: null,
+ i18nErrorOccurred: i18n.error_occurred,
+ i18nErrorMessage: null,
}),
computed: {
@@ -112,14 +118,20 @@ export default {
methods: {
load(data, callback) {
this.loading = true;
- $.get(`${appSubUrl}/api/v1/repos/${data.owner}/${data.repo}/issues/${data.index}`, (issue) => {
+ this.i18nErrorMessage = null;
+ $.get(`${appSubUrl}/api/v1/repos/${data.owner}/${data.repo}/issues/${data.index}`).done((issue) => {
this.issue = issue;
+ }).fail((jqXHR) => {
+ if (jqXHR.responseJSON && jqXHR.responseJSON.message) {
+ this.i18nErrorMessage = jqXHR.responseJSON.message;
+ } else {
+ this.i18nErrorMessage = i18n.network_error;
+ }
+ }).always(() => {
this.loading = false;
- this.$nextTick(() => {
- if (callback) {
- callback();
- }
- });
+ if (callback) {
+ this.$nextTick(callback);
+ }
});
}
}