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>

Changes

6 changed files (+45/-23)

• 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) {
• 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()), ) }
• 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, }) }
• 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
• 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 || {};
• 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); + } }); } }