From a4d78e6e60c262e4058ac63b9eaafae68eab7102 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 15 Nov 2018 21:39:08 +0100 Subject: [PATCH 01/18] improve error handling in the ui --- .../packages/ui-components/package.json | 3 +- .../ui-components/src/ErrorNotification.js | 64 +++++++++++++++++- .../packages/ui-components/src/apiclient.js | 33 ++++++---- .../ui-components/src/apiclient.test.js | 65 ++++++++++++++++++- .../packages/ui-components/src/errors.js | 53 +++++++++++++++ .../packages/ui-components/src/errors.test.js | 35 ++++++++++ .../packages/ui-components/src/index.js | 3 +- .../packages/ui-components/yarn.lock | 23 ++++++- scm-ui/src/groups/modules/groups.js | 26 ++------ scm-ui/src/modules/auth.js | 6 +- scm-ui/src/repos/modules/repos.js | 3 +- scm-ui/src/repos/modules/repositoryTypes.js | 5 +- scm-ui/src/users/modules/users.js | 19 ++---- 13 files changed, 275 insertions(+), 63 deletions(-) create mode 100644 scm-ui-components/packages/ui-components/src/errors.js create mode 100644 scm-ui-components/packages/ui-components/src/errors.test.js diff --git a/scm-ui-components/packages/ui-components/package.json b/scm-ui-components/packages/ui-components/package.json index 823ca7143e..4a4b4dc82e 100644 --- a/scm-ui-components/packages/ui-components/package.json +++ b/scm-ui-components/packages/ui-components/package.json @@ -18,6 +18,7 @@ "create-index": "^2.3.0", "enzyme": "^3.5.0", "enzyme-adapter-react-16": "^1.3.1", + "fetch-mock": "^7.2.5", "flow-bin": "^0.79.1", "flow-typed": "^2.5.1", "jest": "^23.5.0", @@ -55,4 +56,4 @@ ] ] } -} \ No newline at end of file +} diff --git a/scm-ui-components/packages/ui-components/src/ErrorNotification.js b/scm-ui-components/packages/ui-components/src/ErrorNotification.js index 9ef3b58653..6a0dc202eb 100644 --- a/scm-ui-components/packages/ui-components/src/ErrorNotification.js +++ b/scm-ui-components/packages/ui-components/src/ErrorNotification.js @@ -2,6 +2,7 @@ import React from "react"; import { translate } from "react-i18next"; import Notification from "./Notification"; +import { BackendError } from "./errors"; type Props = { t: string => string, @@ -9,12 +10,71 @@ type Props = { }; class ErrorNotification extends React.Component { + + renderMoreInformationsLink(error: BackendError) { + if (error.url) { + // TODO i18n + return ( +

+ For more informations, see {error.errorCode} +

+ ); + } + } + + renderBackendError(error: BackendError) { + // TODO i18n + // how should we handle i18n for the message? + // we could add translation for known error codes to i18n and pass the context objects as parameters, + // but this will not work for errors from plugins, because the ErrorNotification will search for the translation + // in the wrong namespace (plugins could only add translations to the plugins namespace. + // should we add a special namespace for errors? which could be extend by plugins? + + // TODO error page + // we have currently the ErrorNotification, which is often wrapped by the ErrorPage + // the ErrorPage has often a SubTitle like "Unkwown xzy error", which is no longer always the case + // if the error is a BackendError its not fully unknown + return ( +
+

{error.message}

+

Context:

+
    + {error.context.map((context, index) => { + return ( +
  • + {context.type}: {context.id} +
  • + ); + })} +
+ { this.renderMoreInformationsLink(error) } +
+
+ ErrorCode: {error.errorCode} +
+
+ TransactionId: {error.transactionId} +
+
+
+ ); + } + + renderError(error: Error) { + if (error instanceof BackendError) { + return this.renderBackendError(error); + } else { + return error.message; + } + } + render() { - const { t, error } = this.props; + const { error } = this.props; + if (error) { return ( - {t("error-notification.prefix")}: {error.message} + {this.renderError(error)} ); } diff --git a/scm-ui-components/packages/ui-components/src/apiclient.js b/scm-ui-components/packages/ui-components/src/apiclient.js index bd19dcdf14..348371fddd 100644 --- a/scm-ui-components/packages/ui-components/src/apiclient.js +++ b/scm-ui-components/packages/ui-components/src/apiclient.js @@ -1,8 +1,7 @@ // @flow import { contextPath } from "./urls"; - -export const NOT_FOUND_ERROR = Error("not found"); -export const UNAUTHORIZED_ERROR = Error("unauthorized"); +import { createBackendError } from "./errors"; +import type { BackendErrorContent } from "./errors"; const fetchOptions: RequestOptions = { credentials: "same-origin", @@ -11,15 +10,21 @@ const fetchOptions: RequestOptions = { } }; -function handleStatusCode(response: Response) { + +function isBackendError(response) { + return response.headers.get("Content-Type") === "application/vnd.scmm-error+json;v=2"; +} + +function handleFailure(response: Response) { if (!response.ok) { - if (response.status === 401) { - throw UNAUTHORIZED_ERROR; + if (isBackendError(response)) { + return response.json() + .then((content: BackendErrorContent) => { + throw createBackendError(content, response.status); + }); + } else { + throw new Error("server returned status code " + response.status); } - if (response.status === 404) { - throw NOT_FOUND_ERROR; - } - throw new Error("server returned status code " + response.status); } return response; } @@ -37,7 +42,7 @@ export function createUrl(url: string) { class ApiClient { get(url: string): Promise { - return fetch(createUrl(url), fetchOptions).then(handleStatusCode); + return fetch(createUrl(url), fetchOptions).then(handleFailure); } post(url: string, payload: any, contentType: string = "application/json") { @@ -53,7 +58,7 @@ class ApiClient { method: "HEAD" }; options = Object.assign(options, fetchOptions); - return fetch(createUrl(url), options).then(handleStatusCode); + return fetch(createUrl(url), options).then(handleFailure); } delete(url: string): Promise { @@ -61,7 +66,7 @@ class ApiClient { method: "DELETE" }; options = Object.assign(options, fetchOptions); - return fetch(createUrl(url), options).then(handleStatusCode); + return fetch(createUrl(url), options).then(handleFailure); } httpRequestWithJSONBody( @@ -78,7 +83,7 @@ class ApiClient { // $FlowFixMe options.headers["Content-Type"] = contentType; - return fetch(createUrl(url), options).then(handleStatusCode); + return fetch(createUrl(url), options).then(handleFailure); } } diff --git a/scm-ui-components/packages/ui-components/src/apiclient.test.js b/scm-ui-components/packages/ui-components/src/apiclient.test.js index deb22a3b54..8827ca72e4 100644 --- a/scm-ui-components/packages/ui-components/src/apiclient.test.js +++ b/scm-ui-components/packages/ui-components/src/apiclient.test.js @@ -1,5 +1,7 @@ // @flow -import { createUrl } from "./apiclient"; +import { apiClient, createUrl } from "./apiclient"; +import {fetchMock} from "fetch-mock"; +import { BackendError } from "./errors"; describe("create url", () => { it("should not change absolute urls", () => { @@ -13,3 +15,64 @@ describe("create url", () => { expect(createUrl("users")).toBe("/api/v2/users"); }); }); + + +describe("error handling tests", () => { + + const earthNotFoundError = { + transactionId: "42t", + errorCode: "42e", + message: "earth not found", + context: [{ + type: "planet", + id: "earth" + }] + }; + + afterEach(() => { + fetchMock.reset(); + fetchMock.restore(); + }); + + it("should create a normal error, if the content type is not scmm-error", (done) => { + + fetchMock.getOnce("/api/v2/error", { + status: 404 + }); + + apiClient.get("/error") + .catch((err: Error) => { + expect(err.name).toEqual("Error"); + expect(err.message).toContain("404"); + done(); + }); + }); + + it("should create an backend error, if the content type is scmm-error", (done) => { + fetchMock.getOnce("/api/v2/error", { + status: 404, + headers: { + "Content-Type": "application/vnd.scmm-error+json;v=2" + }, + body: earthNotFoundError + }); + + apiClient.get("/error") + .catch((err: BackendError) => { + + expect(err).toBeInstanceOf(BackendError); + + expect(err.message).toEqual("earth not found"); + expect(err.statusCode).toBe(404); + + expect(err.transactionId).toEqual("42t"); + expect(err.errorCode).toEqual("42e"); + expect(err.context).toEqual([{ + type: "planet", + id: "earth" + }]); + done(); + }); + }); + +}); diff --git a/scm-ui-components/packages/ui-components/src/errors.js b/scm-ui-components/packages/ui-components/src/errors.js new file mode 100644 index 0000000000..8becec8a7e --- /dev/null +++ b/scm-ui-components/packages/ui-components/src/errors.js @@ -0,0 +1,53 @@ +// @flow +type Context = {type: string, id: string}[]; + +export type BackendErrorContent = { + transactionId: string, + errorCode: string, + message: string, + url?: string, + context: Context +}; + +export class BackendError extends Error { + + transactionId: string; + errorCode: string; + url: ?string; + context: Context = []; + statusCode: number; + + constructor(content: BackendErrorContent, name: string, statusCode: number) { + super(content.message); + this.name = name; + this.transactionId = content.transactionId; + this.errorCode = content.errorCode; + this.url = content.url; + this.context = content.context; + this.statusCode = statusCode; + } + +} + +export class UnauthorizedError extends BackendError { + constructor(content: BackendErrorContent, statusCode: number) { + super(content, "UnauthorizedError", statusCode); + } +} + +export class NotFoundError extends BackendError { + constructor(content: BackendErrorContent, statusCode: number) { + super(content, "NotFoundError", statusCode); + } +} + +export function createBackendError(content: BackendErrorContent, statusCode: number) { + switch (statusCode) { + case 403: + return new UnauthorizedError(content, statusCode); + case 404: + return new NotFoundError(content, statusCode); + default: + return new BackendError(content, "BackendError", statusCode); + } +} diff --git a/scm-ui-components/packages/ui-components/src/errors.test.js b/scm-ui-components/packages/ui-components/src/errors.test.js new file mode 100644 index 0000000000..3a886e7143 --- /dev/null +++ b/scm-ui-components/packages/ui-components/src/errors.test.js @@ -0,0 +1,35 @@ +// @flow + +import { BackendError, UnauthorizedError, createBackendError, NotFoundError } from "./errors"; + +describe("test createBackendError", () => { + + const earthNotFoundError = { + transactionId: "42t", + errorCode: "42e", + message: "earth not found", + context: [{ + type: "planet", + id: "earth" + }] + }; + + it("should return a default backend error", () => { + const err = createBackendError(earthNotFoundError, 500); + expect(err).toBeInstanceOf(BackendError); + expect(err.name).toBe("BackendError"); + }); + + it("should return an unauthorized error for status code 403", () => { + const err = createBackendError(earthNotFoundError, 403); + expect(err).toBeInstanceOf(UnauthorizedError); + expect(err.name).toBe("UnauthorizedError"); + }); + + it("should return an not found error for status code 404", () => { + const err = createBackendError(earthNotFoundError, 404); + expect(err).toBeInstanceOf(NotFoundError); + expect(err.name).toBe("NotFoundError"); + }); + +}); diff --git a/scm-ui-components/packages/ui-components/src/index.js b/scm-ui-components/packages/ui-components/src/index.js index 41e385af8d..9dc4af1aac 100644 --- a/scm-ui-components/packages/ui-components/src/index.js +++ b/scm-ui-components/packages/ui-components/src/index.js @@ -22,7 +22,8 @@ export { default as HelpIcon } from "./HelpIcon"; export { default as Tooltip } from "./Tooltip"; export { getPageFromMatch } from "./urls"; -export { apiClient, NOT_FOUND_ERROR, UNAUTHORIZED_ERROR } from "./apiclient.js"; +export { apiClient } from "./apiclient.js"; +export * from "./errors"; export * from "./buttons"; export * from "./config"; diff --git a/scm-ui-components/packages/ui-components/yarn.lock b/scm-ui-components/packages/ui-components/yarn.lock index f11cfa5bcd..d7afe51035 100644 --- a/scm-ui-components/packages/ui-components/yarn.lock +++ b/scm-ui-components/packages/ui-components/yarn.lock @@ -688,6 +688,10 @@ react "^16.4.2" react-dom "^16.4.2" +"@scm-manager/ui-types@2.0.0-SNAPSHOT": + version "2.0.0-20181010-130547" + resolved "https://registry.yarnpkg.com/@scm-manager/ui-types/-/ui-types-2.0.0-20181010-130547.tgz#9987b519e43d5c4b895327d012d3fd72429a7953" + "@types/node@*": version "10.12.0" resolved "https://registry.yarnpkg.com/@types/node/-/node-10.12.0.tgz#ea6dcbddbc5b584c83f06c60e82736d8fbb0c235" @@ -2995,6 +2999,15 @@ fb-watchman@^2.0.0: dependencies: bser "^2.0.0" +fetch-mock@^7.2.5: + version "7.2.5" + resolved "https://registry.yarnpkg.com/fetch-mock/-/fetch-mock-7.2.5.tgz#4682f51b9fa74d790e10a471066cb22f3ff84d48" + dependencies: + babel-polyfill "^6.26.0" + glob-to-regexp "^0.4.0" + path-to-regexp "^2.2.1" + whatwg-url "^6.5.0" + figures@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/figures/-/figures-2.0.0.tgz#3ab1a2d2a62c8bfb431a0c94cb797a2fce27c962" @@ -3341,6 +3354,10 @@ glob-stream@^3.1.5: through2 "^0.6.1" unique-stream "^1.0.0" +glob-to-regexp@^0.4.0: + version "0.4.0" + resolved "https://registry.yarnpkg.com/glob-to-regexp/-/glob-to-regexp-0.4.0.tgz#49bd677b1671022bd10921c3788f23cdebf9c7e6" + glob-watcher@^0.0.6: version "0.0.6" resolved "https://registry.yarnpkg.com/glob-watcher/-/glob-watcher-0.0.6.tgz#b95b4a8df74b39c83298b0c05c978b4d9a3b710b" @@ -5982,6 +5999,10 @@ path-to-regexp@^1.7.0: dependencies: isarray "0.0.1" +path-to-regexp@^2.2.1: + version "2.4.0" + resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-2.4.0.tgz#35ce7f333d5616f1c1e1bfe266c3aba2e5b2e704" + path-type@^1.0.0: version "1.1.0" resolved "https://registry.yarnpkg.com/path-type/-/path-type-1.1.0.tgz#59c44f7ee491da704da415da5a4070ba4f8fe441" @@ -7814,7 +7835,7 @@ whatwg-mimetype@^2.1.0: version "2.2.0" resolved "https://registry.yarnpkg.com/whatwg-mimetype/-/whatwg-mimetype-2.2.0.tgz#a3d58ef10b76009b042d03e25591ece89b88d171" -whatwg-url@^6.4.1: +whatwg-url@^6.4.1, whatwg-url@^6.5.0: version "6.5.0" resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-6.5.0.tgz#f2df02bff176fd65070df74ad5ccbb5a199965a8" dependencies: diff --git a/scm-ui/src/groups/modules/groups.js b/scm-ui/src/groups/modules/groups.js index 165648edaa..ad8676fc7b 100644 --- a/scm-ui/src/groups/modules/groups.js +++ b/scm-ui/src/groups/modules/groups.js @@ -54,8 +54,7 @@ export function fetchGroupsByLink(link: string) { .then(data => { dispatch(fetchGroupsSuccess(data)); }) - .catch(cause => { - const error = new Error(`could not fetch groups: ${cause.message}`); + .catch(error => { dispatch(fetchGroupsFailure(link, error)); }); }; @@ -105,8 +104,7 @@ function fetchGroup(link: string, name: string) { .then(data => { dispatch(fetchGroupSuccess(data)); }) - .catch(cause => { - const error = new Error(`could not fetch group: ${cause.message}`); + .catch(error => { dispatch(fetchGroupFailure(name, error)); }); }; @@ -152,11 +150,7 @@ export function createGroup(link: string, group: Group, callback?: () => void) { } }) .catch(error => { - dispatch( - createGroupFailure( - new Error(`Failed to create group ${group.name}: ${error.message}`) - ) - ); + dispatch(createGroupFailure(error)); }); }; } @@ -201,13 +195,8 @@ export function modifyGroup(group: Group, callback?: () => void) { .then(() => { dispatch(fetchGroupByLink(group)); }) - .catch(cause => { - dispatch( - modifyGroupFailure( - group, - new Error(`could not modify group ${group.name}: ${cause.message}`) - ) - ); + .catch(error => { + dispatch(modifyGroupFailure(group, error)); }); }; } @@ -259,10 +248,7 @@ export function deleteGroup(group: Group, callback?: () => void) { callback(); } }) - .catch(cause => { - const error = new Error( - `could not delete group ${group.name}: ${cause.message}` - ); + .catch(error => { dispatch(deleteGroupFailure(group, error)); }); }; diff --git a/scm-ui/src/modules/auth.js b/scm-ui/src/modules/auth.js index 691ae2b128..6dcd03847b 100644 --- a/scm-ui/src/modules/auth.js +++ b/scm-ui/src/modules/auth.js @@ -2,7 +2,7 @@ import type { Me } from "@scm-manager/ui-types"; import * as types from "./types"; -import { apiClient, UNAUTHORIZED_ERROR } from "@scm-manager/ui-components"; +import { apiClient, UnauthorizedError } from "@scm-manager/ui-components"; import { isPending } from "./pending"; import { getFailure } from "./failure"; import { @@ -159,7 +159,7 @@ export const login = ( dispatch(loginPending()); return apiClient .post(loginLink, login_data) - .then(response => { + .then(() => { dispatch(fetchIndexResourcesPending()); return callFetchIndexResources(); }) @@ -185,7 +185,7 @@ export const fetchMe = (link: string) => { dispatch(fetchMeSuccess(me)); }) .catch((error: Error) => { - if (error === UNAUTHORIZED_ERROR) { + if (error instanceof UnauthorizedError) { dispatch(fetchMeUnauthenticated()); } else { dispatch(fetchMeFailure(error)); diff --git a/scm-ui/src/repos/modules/repos.js b/scm-ui/src/repos/modules/repos.js index 642f6cf395..dd5985d444 100644 --- a/scm-ui/src/repos/modules/repos.js +++ b/scm-ui/src/repos/modules/repos.js @@ -224,8 +224,7 @@ export function modifyRepo(repository: Repository, callback?: () => void) { .then(() => { dispatch(fetchRepoByLink(repository)); }) - .catch(cause => { - const error = new Error(`failed to modify repo: ${cause.message}`); + .catch(error => { dispatch(modifyRepoFailure(repository, error)); }); }; diff --git a/scm-ui/src/repos/modules/repositoryTypes.js b/scm-ui/src/repos/modules/repositoryTypes.js index 2cdbd1194d..d96d24b612 100644 --- a/scm-ui/src/repos/modules/repositoryTypes.js +++ b/scm-ui/src/repos/modules/repositoryTypes.js @@ -37,10 +37,7 @@ function fetchRepositoryTypes(dispatch: any) { .then(repositoryTypes => { dispatch(fetchRepositoryTypesSuccess(repositoryTypes)); }) - .catch(err => { - const error = new Error( - `failed to fetch repository types: ${err.message}` - ); + .catch(error => { dispatch(fetchRepositoryTypesFailure(error)); }); } diff --git a/scm-ui/src/users/modules/users.js b/scm-ui/src/users/modules/users.js index fe751d13d4..a93d082f58 100644 --- a/scm-ui/src/users/modules/users.js +++ b/scm-ui/src/users/modules/users.js @@ -57,8 +57,7 @@ export function fetchUsersByLink(link: string) { .then(data => { dispatch(fetchUsersSuccess(data)); }) - .catch(cause => { - const error = new Error(`could not fetch users: ${cause.message}`); + .catch(error => { dispatch(fetchUsersFailure(link, error)); }); }; @@ -108,8 +107,7 @@ function fetchUser(link: string, name: string) { .then(data => { dispatch(fetchUserSuccess(data)); }) - .catch(cause => { - const error = new Error(`could not fetch user: ${cause.message}`); + .catch(error => { dispatch(fetchUserFailure(name, error)); }); }; @@ -155,12 +153,8 @@ export function createUser(link: string, user: User, callback?: () => void) { callback(); } }) - .catch(err => - dispatch( - createUserFailure( - new Error(`failed to add user ${user.name}: ${err.message}`) - ) - ) + .catch(error => + dispatch(createUserFailure(error)) ); }; } @@ -260,10 +254,7 @@ export function deleteUser(user: User, callback?: () => void) { callback(); } }) - .catch(cause => { - const error = new Error( - `could not delete user ${user.name}: ${cause.message}` - ); + .catch(error => { dispatch(deleteUserFailure(user, error)); }); }; From bbde074e196b257475addaac429032a3ac76a32a Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Mon, 25 Feb 2019 15:09:28 +0100 Subject: [PATCH 02/18] Fixed unit test --- .../packages/ui-components/src/errors.js | 2 +- scm-ui-components/packages/ui-components/yarn.lock | 4 ---- scm-ui/src/modules/auth.test.js | 11 +++++++++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/scm-ui-components/packages/ui-components/src/errors.js b/scm-ui-components/packages/ui-components/src/errors.js index 8becec8a7e..420f8d6fad 100644 --- a/scm-ui-components/packages/ui-components/src/errors.js +++ b/scm-ui-components/packages/ui-components/src/errors.js @@ -43,7 +43,7 @@ export class NotFoundError extends BackendError { export function createBackendError(content: BackendErrorContent, statusCode: number) { switch (statusCode) { - case 403: + case 401: return new UnauthorizedError(content, statusCode); case 404: return new NotFoundError(content, statusCode); diff --git a/scm-ui-components/packages/ui-components/yarn.lock b/scm-ui-components/packages/ui-components/yarn.lock index d7afe51035..94816787ec 100644 --- a/scm-ui-components/packages/ui-components/yarn.lock +++ b/scm-ui-components/packages/ui-components/yarn.lock @@ -688,10 +688,6 @@ react "^16.4.2" react-dom "^16.4.2" -"@scm-manager/ui-types@2.0.0-SNAPSHOT": - version "2.0.0-20181010-130547" - resolved "https://registry.yarnpkg.com/@scm-manager/ui-types/-/ui-types-2.0.0-20181010-130547.tgz#9987b519e43d5c4b895327d012d3fd72429a7953" - "@types/node@*": version "10.12.0" resolved "https://registry.yarnpkg.com/@types/node/-/node-10.12.0.tgz#ea6dcbddbc5b584c83f06c60e82736d8fbb0c235" diff --git a/scm-ui/src/modules/auth.test.js b/scm-ui/src/modules/auth.test.js index 7236f803a8..e3b4697ff7 100644 --- a/scm-ui/src/modules/auth.test.js +++ b/scm-ui/src/modules/auth.test.js @@ -180,8 +180,15 @@ describe("auth actions", () => { it("should dispatch fetch me unauthorized", () => { fetchMock.getOnce("/api/v2/me", { - status: 401 - }); + status: 401, + headers: { + "content-type": "application/vnd.scmm-error+json;v=2" + }, + body: { + transactionId: "123", + message: "So nicht Freundchen", + context: {} + }}); const expectedActions = [ { type: FETCH_ME_PENDING }, From aac94d17a32fc57541a0b19ab14c94174f3abe1d Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Mon, 25 Feb 2019 17:40:53 +0100 Subject: [PATCH 03/18] Fixed 401 message on login --- .../packages/ui-components/src/apiclient.js | 9 +++---- .../packages/ui-components/src/errors.js | 24 +++++++++++-------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/scm-ui-components/packages/ui-components/src/apiclient.js b/scm-ui-components/packages/ui-components/src/apiclient.js index 0d52167d95..ce7de1b757 100644 --- a/scm-ui-components/packages/ui-components/src/apiclient.js +++ b/scm-ui-components/packages/ui-components/src/apiclient.js @@ -1,6 +1,6 @@ // @flow import { contextPath } from "./urls"; -import { createBackendError } from "./errors"; +import {createBackendError, isBackendError, UnauthorizedError} from "./errors"; import type { BackendErrorContent } from "./errors"; const fetchOptions: RequestOptions = { @@ -10,9 +10,7 @@ const fetchOptions: RequestOptions = { } }; -function isBackendError(response) { - return response.headers.get("Content-Type") === "application/vnd.scmm-error+json;v=2"; -} + function handleFailure(response: Response) { if (!response.ok) { @@ -22,6 +20,9 @@ function handleFailure(response: Response) { throw createBackendError(content, response.status); }); } else { + if (response.status === 401) { + throw new UnauthorizedError("Unauthorized", 401); + } throw new Error("server returned status code " + response.status); } } diff --git a/scm-ui-components/packages/ui-components/src/errors.js b/scm-ui-components/packages/ui-components/src/errors.js index 420f8d6fad..9b278c7ddd 100644 --- a/scm-ui-components/packages/ui-components/src/errors.js +++ b/scm-ui-components/packages/ui-components/src/errors.js @@ -1,5 +1,5 @@ // @flow -type Context = {type: string, id: string}[]; +type Context = { type: string, id: string }[]; export type BackendErrorContent = { transactionId: string, @@ -10,7 +10,6 @@ export type BackendErrorContent = { }; export class BackendError extends Error { - transactionId: string; errorCode: string; url: ?string; @@ -26,12 +25,13 @@ export class BackendError extends Error { this.context = content.context; this.statusCode = statusCode; } - } -export class UnauthorizedError extends BackendError { - constructor(content: BackendErrorContent, statusCode: number) { - super(content, "UnauthorizedError", statusCode); +export class UnauthorizedError extends Error { + statusCode: number; + constructor(message: string, statusCode: number) { + super(message); + this.statusCode = statusCode; } } @@ -40,14 +40,18 @@ export class NotFoundError extends BackendError { super(content, "NotFoundError", statusCode); } } - -export function createBackendError(content: BackendErrorContent, statusCode: number) { +export function createBackendError( + content: BackendErrorContent, + statusCode: number +) { switch (statusCode) { - case 401: - return new UnauthorizedError(content, statusCode); case 404: return new NotFoundError(content, statusCode); default: return new BackendError(content, "BackendError", statusCode); } } + +export function isBackendError(response: Response) { + return response.headers.get("Content-Type") === "application/vnd.scmm-error+json;v=2"; +} From 264eca4b496d7a966317886f3ca0806d51daa986 Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Tue, 26 Feb 2019 08:37:00 +0100 Subject: [PATCH 04/18] Fixed 401 message on session timeout --- .../ui-components/src/ErrorNotification.js | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/scm-ui-components/packages/ui-components/src/ErrorNotification.js b/scm-ui-components/packages/ui-components/src/ErrorNotification.js index 64a31c8da4..747978297b 100644 --- a/scm-ui-components/packages/ui-components/src/ErrorNotification.js +++ b/scm-ui-components/packages/ui-components/src/ErrorNotification.js @@ -2,7 +2,7 @@ import React from "react"; import { translate } from "react-i18next"; import Notification from "./Notification"; -import {BackendError, UnauthorizedError} from "./errors"; +import { BackendError, UnauthorizedError } from "./errors"; type Props = { t: string => string, @@ -10,13 +10,15 @@ type Props = { }; class ErrorNotification extends React.Component { - renderMoreInformationLink(error: BackendError) { if (error.url) { // TODO i18n return (

- For more information, see {error.errorCode} + For more information, see{" "} + + {error.errorCode} +

); } @@ -37,7 +39,9 @@ class ErrorNotification extends React.Component { return (

{error.message}

-

Context:

+

+ Context: +

    {error.context.map((context, index) => { return ( @@ -47,24 +51,30 @@ class ErrorNotification extends React.Component { ); })}
- { this.renderMoreInformationLink(error) } + {this.renderMoreInformationLink(error)}
-
- ErrorCode: {error.errorCode} -
-
- TransactionId: {error.transactionId} -
+
ErrorCode: {error.errorCode}
+
TransactionId: {error.transactionId}
); - } + }; render() { const { t, error } = this.props; if (error) { if (error instanceof BackendError) { - return this.renderBackendError(error) + return this.renderBackendError(error); + } else if (error instanceof UnauthorizedError) { + return ( + + {t("error-notification.prefix")}:{" "} + {t("error-notification.timeout")}{" "} + + {t("error-notification.loginLink")} + + + ); } else { return ( From c67b2c31973dc58e0d0c693ba6af15c363488dd6 Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Tue, 26 Feb 2019 11:30:16 +0100 Subject: [PATCH 05/18] Fixed unit test --- scm-ui/src/modules/auth.test.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/scm-ui/src/modules/auth.test.js b/scm-ui/src/modules/auth.test.js index e3b4697ff7..1d7cb5096e 100644 --- a/scm-ui/src/modules/auth.test.js +++ b/scm-ui/src/modules/auth.test.js @@ -179,16 +179,7 @@ describe("auth actions", () => { }); it("should dispatch fetch me unauthorized", () => { - fetchMock.getOnce("/api/v2/me", { - status: 401, - headers: { - "content-type": "application/vnd.scmm-error+json;v=2" - }, - body: { - transactionId: "123", - message: "So nicht Freundchen", - context: {} - }}); + fetchMock.getOnce("/api/v2/me", 401); const expectedActions = [ { type: FETCH_ME_PENDING }, From 6b55df464cf37aa48563d261f864e6f3bdd1c2ac Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Tue, 26 Feb 2019 17:08:33 +0100 Subject: [PATCH 06/18] WIP: Reworking ErrorPage/ErrorNotification --- .../src/BackendErrorNotification.js | 95 +++++++++++++++++++ .../ui-components/src/CollapsibleErrorPage.js | 44 +++++++++ .../ui-components/src/ErrorNotification.js | 56 +---------- .../packages/ui-components/src/index.js | 1 + scm-ui/src/repos/containers/RepositoryRoot.js | 15 +-- .../main/resources/locales/de/plugins.json | 3 + .../main/resources/locales/en/plugins.json | 3 + 7 files changed, 159 insertions(+), 58 deletions(-) create mode 100644 scm-ui-components/packages/ui-components/src/BackendErrorNotification.js create mode 100644 scm-ui-components/packages/ui-components/src/CollapsibleErrorPage.js diff --git a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js new file mode 100644 index 0000000000..cf4d0def0d --- /dev/null +++ b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js @@ -0,0 +1,95 @@ +// @flow +import React from "react"; +import { BackendError } from "./errors"; +import classNames from "classnames"; + +import { translate } from "react-i18next"; + +type Props = { error: BackendError, t: string => string }; +type State = { collapsed: boolean }; + +class BackendErrorNotification extends React.Component { + constructor(props: Props) { + super(props); + this.state = { collapsed: true }; + } + + render() { + const { collapsed } = this.state; + const icon = collapsed ? "fa-angle-right" : "fa-angle-down"; + + // TODO error page + // we have currently the ErrorNotification, which is often wrapped by the ErrorPage + // the ErrorPage has often a SubTitle like "Unkwown xzy error", which is no longer always the case + // if the error is a BackendError its not fully unknown + return ( +
+

+ { + this.setState({ collapsed: !this.state.collapsed }); + }} + > + + + {this.renderErrorMessage()} + +

+ {this.renderUncollapsed()} +
+ ); + } + + renderErrorMessage = () => { + const { error, t } = this.props; + const translation = t("errors." + error.errorCode); + if (translation === error.errorCode) { + return error.message; + } + return translation; + }; + + renderUncollapsed = () => { + const { error } = this.props; + if (!this.state.collapsed) { + return ( + <> +

+ Context: +

+
    + {error.context.map((context, index) => { + return ( +
  • + {context.type}: {context.id} +
  • + ); + })} +
+ {this.renderMoreInformationLink(error)} +
+
ErrorCode: {error.errorCode}
+
TransactionId: {error.transactionId}
+
+ + ); + } + return null; + }; + + renderMoreInformationLink = (error: BackendError) => { + if (error.url) { + // TODO i18n + return ( +

+ For more information, see{" "} + + {error.errorCode} + +

+ ); + } + }; +} + +export default translate("plugins")(BackendErrorNotification); diff --git a/scm-ui-components/packages/ui-components/src/CollapsibleErrorPage.js b/scm-ui-components/packages/ui-components/src/CollapsibleErrorPage.js new file mode 100644 index 0000000000..c7c328fe6d --- /dev/null +++ b/scm-ui-components/packages/ui-components/src/CollapsibleErrorPage.js @@ -0,0 +1,44 @@ +//@flow +import React from "react"; +import ErrorNotification from "./ErrorNotification"; +import { translate } from "react-i18next"; + +type Props = { + error: Error, + title: string, + subtitle?: string, + t: string => string +}; + +type State = { + collapsed: boolean +}; + +class CollapsibleErrorPage extends React.Component { + constructor(props: Props) { + super(props); + this.state = { + collapsed: true + }; + } + + + + render() { + const { title, error, t } = this.props; + + return ( +
+
+ {this.setState({collapsed: !this.state.collapsed})}}> +

{title}

+
+ + +
+
+ ); + } +} + +export default translate("plugins")(CollapsibleErrorPage); diff --git a/scm-ui-components/packages/ui-components/src/ErrorNotification.js b/scm-ui-components/packages/ui-components/src/ErrorNotification.js index 747978297b..ab7d8ad882 100644 --- a/scm-ui-components/packages/ui-components/src/ErrorNotification.js +++ b/scm-ui-components/packages/ui-components/src/ErrorNotification.js @@ -1,70 +1,22 @@ //@flow import React from "react"; import { translate } from "react-i18next"; -import Notification from "./Notification"; import { BackendError, UnauthorizedError } from "./errors"; +import Notification from "./Notification"; +import BackendErrorNotification from "./BackendErrorNotification"; type Props = { t: string => string, error?: Error }; + class ErrorNotification extends React.Component { - renderMoreInformationLink(error: BackendError) { - if (error.url) { - // TODO i18n - return ( -

- For more information, see{" "} - - {error.errorCode} - -

- ); - } - } - - renderBackendError = (error: BackendError) => { - // TODO i18n - // how should we handle i18n for the message? - // we could add translation for known error codes to i18n and pass the context objects as parameters, - // but this will not work for errors from plugins, because the ErrorNotification will search for the translation - // in the wrong namespace (plugins could only add translations to the plugins namespace. - // should we add a special namespace for errors? which could be extend by plugins? - - // TODO error page - // we have currently the ErrorNotification, which is often wrapped by the ErrorPage - // the ErrorPage has often a SubTitle like "Unkwown xzy error", which is no longer always the case - // if the error is a BackendError its not fully unknown - return ( -
-

{error.message}

-

- Context: -

-
    - {error.context.map((context, index) => { - return ( -
  • - {context.type}: {context.id} -
  • - ); - })} -
- {this.renderMoreInformationLink(error)} -
-
ErrorCode: {error.errorCode}
-
TransactionId: {error.transactionId}
-
-
- ); - }; - render() { const { t, error } = this.props; if (error) { if (error instanceof BackendError) { - return this.renderBackendError(error); + return } else if (error instanceof UnauthorizedError) { return ( diff --git a/scm-ui-components/packages/ui-components/src/index.js b/scm-ui-components/packages/ui-components/src/index.js index b403ea78b7..4bf6c57338 100644 --- a/scm-ui-components/packages/ui-components/src/index.js +++ b/scm-ui-components/packages/ui-components/src/index.js @@ -9,6 +9,7 @@ export { validation, urls, repositories }; export { default as DateFromNow } from "./DateFromNow.js"; export { default as ErrorNotification } from "./ErrorNotification.js"; export { default as ErrorPage } from "./ErrorPage.js"; +export { default as CollapsibleErrorPage } from "./CollapsibleErrorPage.js"; export { default as Image } from "./Image.js"; export { default as Loading } from "./Loading.js"; export { default as Logo } from "./Logo.js"; diff --git a/scm-ui/src/repos/containers/RepositoryRoot.js b/scm-ui/src/repos/containers/RepositoryRoot.js index 8608b9c957..e8874675d5 100644 --- a/scm-ui/src/repos/containers/RepositoryRoot.js +++ b/scm-ui/src/repos/containers/RepositoryRoot.js @@ -12,7 +12,7 @@ import { Route, Switch } from "react-router-dom"; import type { Repository } from "@scm-manager/ui-types"; import { - ErrorPage, + CollapsibleErrorPage, Loading, Navigation, SubNavigation, @@ -81,13 +81,16 @@ class RepositoryRoot extends React.Component { render() { const { loading, error, indexLinks, repository, t } = this.props; + // if (error) { + // return + // } if (error) { return ( - + ); } diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index 8c2015a0e8..1f2dcc9a41 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -87,5 +87,8 @@ "description": "Darf im Repository Kontext alles ausführen. Dies beinhaltet alle Repository Berechtigungen." } } + }, + "errors": { + "AGR7UzkhA1": "Nicht gefunden" } } diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index 03eb06e597..6b1081f2d1 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -87,5 +87,8 @@ "description": "May change everything for the repository (includes all other permissions)" } } + }, + "errors": { + "AGR7UzkhA1": "Not found" } } From 510c295e004db06897f39e1d3825a538ad28a136 Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Wed, 27 Feb 2019 10:45:10 +0100 Subject: [PATCH 07/18] Refactored ErrorNotification/ErrorPage Added i18n for error codes --- .../src/BackendErrorNotification.js | 18 +++---- .../ui-components/src/CollapsibleErrorPage.js | 44 --------------- .../ui-components/src/ErrorNotification.js | 2 +- .../packages/ui-components/src/ErrorPage.js | 13 ++++- .../packages/ui-components/src/index.js | 1 - scm-ui/src/config/containers/GlobalConfig.js | 53 ++++++++++++------- scm-ui/src/repos/containers/RepositoryRoot.js | 17 +++--- .../main/resources/locales/de/plugins.json | 10 +++- .../main/resources/locales/en/plugins.json | 10 +++- 9 files changed, 77 insertions(+), 91 deletions(-) delete mode 100644 scm-ui-components/packages/ui-components/src/CollapsibleErrorPage.js diff --git a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js index cf4d0def0d..7540b17030 100644 --- a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js +++ b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js @@ -18,13 +18,9 @@ class BackendErrorNotification extends React.Component { const { collapsed } = this.state; const icon = collapsed ? "fa-angle-right" : "fa-angle-down"; - // TODO error page - // we have currently the ErrorNotification, which is often wrapped by the ErrorPage - // the ErrorPage has often a SubTitle like "Unkwown xzy error", which is no longer always the case - // if the error is a BackendError its not fully unknown return (
-

+

{ this.setState({ collapsed: !this.state.collapsed }); @@ -50,12 +46,12 @@ class BackendErrorNotification extends React.Component { }; renderUncollapsed = () => { - const { error } = this.props; + const { error, t } = this.props; if (!this.state.collapsed) { return ( <>

- Context: + {t("errors.context")}

    {error.context.map((context, index) => { @@ -68,8 +64,8 @@ class BackendErrorNotification extends React.Component {
{this.renderMoreInformationLink(error)}
-
ErrorCode: {error.errorCode}
-
TransactionId: {error.transactionId}
+
{t("errors.errorCode")} {error.errorCode}
+
{t("errors.transactionId")} {error.transactionId}
); @@ -78,11 +74,11 @@ class BackendErrorNotification extends React.Component { }; renderMoreInformationLink = (error: BackendError) => { + const { t } = this.props; if (error.url) { - // TODO i18n return (

- For more information, see{" "} + {t("errors.moreInfo")}{" "} {error.errorCode} diff --git a/scm-ui-components/packages/ui-components/src/CollapsibleErrorPage.js b/scm-ui-components/packages/ui-components/src/CollapsibleErrorPage.js deleted file mode 100644 index c7c328fe6d..0000000000 --- a/scm-ui-components/packages/ui-components/src/CollapsibleErrorPage.js +++ /dev/null @@ -1,44 +0,0 @@ -//@flow -import React from "react"; -import ErrorNotification from "./ErrorNotification"; -import { translate } from "react-i18next"; - -type Props = { - error: Error, - title: string, - subtitle?: string, - t: string => string -}; - -type State = { - collapsed: boolean -}; - -class CollapsibleErrorPage extends React.Component { - constructor(props: Props) { - super(props); - this.state = { - collapsed: true - }; - } - - - - render() { - const { title, error, t } = this.props; - - return ( -

-
- {this.setState({collapsed: !this.state.collapsed})}}> -

{title}

-
- - -
-
- ); - } -} - -export default translate("plugins")(CollapsibleErrorPage); diff --git a/scm-ui-components/packages/ui-components/src/ErrorNotification.js b/scm-ui-components/packages/ui-components/src/ErrorNotification.js index ab7d8ad882..380ea04082 100644 --- a/scm-ui-components/packages/ui-components/src/ErrorNotification.js +++ b/scm-ui-components/packages/ui-components/src/ErrorNotification.js @@ -39,4 +39,4 @@ class ErrorNotification extends React.Component { } } -export default translate("commons")(ErrorNotification); +export default translate("commons")(ErrorNotification); diff --git a/scm-ui-components/packages/ui-components/src/ErrorPage.js b/scm-ui-components/packages/ui-components/src/ErrorPage.js index 196319681c..8947e702c8 100644 --- a/scm-ui-components/packages/ui-components/src/ErrorPage.js +++ b/scm-ui-components/packages/ui-components/src/ErrorPage.js @@ -1,6 +1,7 @@ //@flow import React from "react"; import ErrorNotification from "./ErrorNotification"; +import { BackendError } from "./errors"; type Props = { error: Error, @@ -10,18 +11,26 @@ type Props = { class ErrorPage extends React.Component { render() { - const { title, subtitle, error } = this.props; + const { title, error } = this.props; return (

{title}

-

{subtitle}

+ {this.renderSubtitle()}
); } + + renderSubtitle = () => { + const { error, subtitle } = this.props; + if (error instanceof BackendError) { + return null; + } + return

{subtitle}

+ } } export default ErrorPage; diff --git a/scm-ui-components/packages/ui-components/src/index.js b/scm-ui-components/packages/ui-components/src/index.js index 4bf6c57338..b403ea78b7 100644 --- a/scm-ui-components/packages/ui-components/src/index.js +++ b/scm-ui-components/packages/ui-components/src/index.js @@ -9,7 +9,6 @@ export { validation, urls, repositories }; export { default as DateFromNow } from "./DateFromNow.js"; export { default as ErrorNotification } from "./ErrorNotification.js"; export { default as ErrorPage } from "./ErrorPage.js"; -export { default as CollapsibleErrorPage } from "./CollapsibleErrorPage.js"; export { default as Image } from "./Image.js"; export { default as Loading } from "./Loading.js"; export { default as Logo } from "./Logo.js"; diff --git a/scm-ui/src/config/containers/GlobalConfig.js b/scm-ui/src/config/containers/GlobalConfig.js index eac8e27bee..de01e98aab 100644 --- a/scm-ui/src/config/containers/GlobalConfig.js +++ b/scm-ui/src/config/containers/GlobalConfig.js @@ -1,7 +1,11 @@ // @flow import React from "react"; import { translate } from "react-i18next"; -import { Title, ErrorPage, Loading } from "@scm-manager/ui-components"; +import { + Title, + Loading, + ErrorNotification +} from "@scm-manager/ui-components"; import { fetchConfig, getFetchConfigFailure, @@ -73,18 +77,8 @@ class GlobalConfig extends React.Component { }; render() { - const { t, error, loading, config, configUpdatePermission } = this.props; + const { t, loading } = this.props; - if (error) { - return ( - - ); - } if (loading) { return ; } @@ -92,16 +86,37 @@ class GlobalConfig extends React.Component { return (
- {this.renderConfigChangedNotification()} - <ConfigForm - submitForm={config => this.modifyConfig(config)} - config={config} - loading={loading} - configUpdatePermission={configUpdatePermission} - /> + {this.renderError()} + {this.renderContent()} </div> ); } + + renderError = () => { + const { error } = this.props; + if (error) { + return <ErrorNotification error={error} />; + } + return null; + }; + + renderContent = () => { + const { error, loading, config, configUpdatePermission } = this.props; + if (!error) { + return ( + <> + {this.renderConfigChangedNotification()} + <ConfigForm + submitForm={config => this.modifyConfig(config)} + config={config} + loading={loading} + configUpdatePermission={configUpdatePermission} + /> + </> + ); + } + return null; + }; } const mapDispatchToProps = dispatch => { diff --git a/scm-ui/src/repos/containers/RepositoryRoot.js b/scm-ui/src/repos/containers/RepositoryRoot.js index e8874675d5..08d6aa9380 100644 --- a/scm-ui/src/repos/containers/RepositoryRoot.js +++ b/scm-ui/src/repos/containers/RepositoryRoot.js @@ -18,7 +18,7 @@ import { SubNavigation, NavLink, Page, - Section + Section, ErrorPage } from "@scm-manager/ui-components"; import { translate } from "react-i18next"; import RepositoryDetails from "../components/RepositoryDetails"; @@ -81,17 +81,12 @@ class RepositoryRoot extends React.Component<Props> { render() { const { loading, error, indexLinks, repository, t } = this.props; - // if (error) { - // return <ErrorPage - // title={t("repositoryRoot.errorTitle")} - // subtitle={t("repositoryRoot.errorSubtitle")} - // error={error} - // /> - // } if (error) { - return ( - <CollapsibleErrorPage title={t("repositoryRoot.errorTitle")} error={error} /> - ); + return <ErrorPage + title={t("repositoryRoot.errorTitle")} + subtitle={t("repositoryRoot.errorSubtitle")} + error={error} + /> } if (!repository || loading) { diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index 1f2dcc9a41..159194c0dc 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -89,6 +89,14 @@ } }, "errors": { - "AGR7UzkhA1": "Nicht gefunden" + "context": "Kontext", + "errorCode": "Fehlercode", + "transactionId": "Transaktions-ID", + "moreInfo": "Für mehr Informationen, siehe", + "AGR7UzkhA1": "Nicht gefunden", + "FtR7UznKU1": "Existiert bereits", + "9BR7qpDAe1": "Passwortänderung nicht erlaubt", + "2wR7UzpPG1": "Konkurrierende Änderungen", + "9SR8G0kmU1": "Feature nicht unterstützt" } } diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index 6b1081f2d1..6319c5012e 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -89,6 +89,14 @@ } }, "errors": { - "AGR7UzkhA1": "Not found" + "context": "context", + "errorCode": "Error Code", + "transactionId": "Transaction ID", + "moreInfo": "For more information, see", + "AGR7UzkhA1": "Not found", + "FtR7UznKU1": "Already exists", + "9BR7qpDAe1": "Password change not allowed", + "2wR7UzpPG1": "Concurrent modifications", + "9SR8G0kmU1": "Feature not supported" } } From 1b7044ed762252917d38f86d292bd9d8a958e088 Mon Sep 17 00:00:00 2001 From: Philipp Czora <philipp.czora@cloudogu.com> Date: Wed, 27 Feb 2019 14:28:20 +0100 Subject: [PATCH 08/18] Fixed Notification color --- .../packages/ui-components/src/BackendErrorNotification.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js index 7540b17030..dc38208be0 100644 --- a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js +++ b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js @@ -1,9 +1,10 @@ // @flow import React from "react"; -import { BackendError } from "./errors"; +import {BackendError} from "./errors"; import classNames from "classnames"; +import Notification from "./Notification"; -import { translate } from "react-i18next"; +import {translate} from "react-i18next"; type Props = { error: BackendError, t: string => string }; type State = { collapsed: boolean }; @@ -19,6 +20,7 @@ class BackendErrorNotification extends React.Component<Props, State> { const icon = collapsed ? "fa-angle-right" : "fa-angle-down"; return ( + <Notification type="danger"> <div className="content"> <p className="subtitle"> <span @@ -33,6 +35,7 @@ class BackendErrorNotification extends React.Component<Props, State> { </p> {this.renderUncollapsed()} </div> + </Notification> ); } From 5d375b7fb45518bb3298106ce99c3d62c8d13e15 Mon Sep 17 00:00:00 2001 From: Philipp Czora <philipp.czora@cloudogu.com> Date: Fri, 1 Mar 2019 13:16:54 +0100 Subject: [PATCH 09/18] Added error descriptions & removed collapsability (according to review) --- .../src/BackendErrorNotification.js | 93 +++++++++---------- .../main/resources/locales/de/plugins.json | 25 ++++- .../main/resources/locales/en/plugins.json | 27 ++++-- 3 files changed, 85 insertions(+), 60 deletions(-) diff --git a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js index dc38208be0..0995417e57 100644 --- a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js +++ b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js @@ -1,79 +1,74 @@ // @flow import React from "react"; -import {BackendError} from "./errors"; -import classNames from "classnames"; +import { BackendError } from "./errors"; import Notification from "./Notification"; -import {translate} from "react-i18next"; +import { translate } from "react-i18next"; type Props = { error: BackendError, t: string => string }; -type State = { collapsed: boolean }; -class BackendErrorNotification extends React.Component<Props, State> { +class BackendErrorNotification extends React.Component<Props> { constructor(props: Props) { super(props); - this.state = { collapsed: true }; } render() { - const { collapsed } = this.state; - const icon = collapsed ? "fa-angle-right" : "fa-angle-down"; - return ( <Notification type="danger"> - <div className="content"> - <p className="subtitle"> - <span - onClick={() => { - this.setState({ collapsed: !this.state.collapsed }); - }} - > - <i className={classNames("fa", icon)} /> - - {this.renderErrorMessage()} - </span> - </p> - {this.renderUncollapsed()} - </div> + <div className="content"> + <p className="subtitle">{this.renderErrorName()}</p> + <p>{this.renderErrorDescription()}</p> + {this.renderMetadata()} + </div> </Notification> ); } - renderErrorMessage = () => { + renderErrorName = () => { const { error, t } = this.props; - const translation = t("errors." + error.errorCode); + const translation = t("errors." + error.errorCode + ".displayName"); if (translation === error.errorCode) { return error.message; } return translation; }; - renderUncollapsed = () => { + renderErrorDescription = () => { const { error, t } = this.props; - if (!this.state.collapsed) { - return ( - <> - <p> - <strong>{t("errors.context")}</strong> - </p> - <ul> - {error.context.map((context, index) => { - return ( - <li key={index}> - <strong>{context.type}:</strong> {context.id} - </li> - ); - })} - </ul> - {this.renderMoreInformationLink(error)} - <div className="level is-size-7"> - <div className="left">{t("errors.errorCode")} {error.errorCode}</div> - <div className="right">{t("errors.transactionId")} {error.transactionId}</div> - </div> - </> - ); + const translation = t("errors." + error.errorCode + ".description"); + if (translation === error.errorCode) { + return ""; } - return null; + return translation; + }; + + renderMetadata = () => { + const { error, t } = this.props; + return ( + <> + <p> + <strong>{t("errors.context")}</strong> + </p> + <ul> + {error.context.map((context, index) => { + return ( + <li key={index}> + <strong>{context.type}:</strong> {context.id} + </li> + ); + })} + </ul> + {this.renderMoreInformationLink(error)} + <div className="level is-size-7"> + <div className="left"> + {t("errors.transactionId")} {error.transactionId} + </div> + <div className="right"> + {t("errors.errorCode")} {error.errorCode} + </div> + </div> + </> + ); }; renderMoreInformationLink = (error: BackendError) => { diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index 159194c0dc..2f619f0b8a 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -93,10 +93,25 @@ "errorCode": "Fehlercode", "transactionId": "Transaktions-ID", "moreInfo": "Für mehr Informationen, siehe", - "AGR7UzkhA1": "Nicht gefunden", - "FtR7UznKU1": "Existiert bereits", - "9BR7qpDAe1": "Passwortänderung nicht erlaubt", - "2wR7UzpPG1": "Konkurrierende Änderungen", - "9SR8G0kmU1": "Feature nicht unterstützt" + "AGR7UzkhA1": { + "displayName": "Nicht gefunden", + "description": "Die angefragte Entität konnte nicht gefunden werden. Möglicherweise wurde es in einer weiteren Session gelöscht." + }, + "FtR7UznKU1": { + "displayName": "Existiert bereits", + "description": "Eine Entität mit den gegebenen Schlüsselwerten existiert bereits" + }, + "9BR7qpDAe1": { + "displayName": "Passwortänderung nicht erlaubt", + "description": "Sie haben nicht die Berechtigung, das Passwort zu ändern" + }, + "2wR7UzpPG1": { + "displayName": "Konkurrierende Änderungen", + "description": "Die Entität wurde konkurrierend von einem anderen Benutzer oder einem anderen Prozess modifiziert. Bitte laden sie die Entität erneut." + }, + "9SR8G0kmU1": { + "displayName": "Feature nicht unterstützt", + "description": "Das Versionsverwaltungssystem dieses Repo unterstützt das angefragte Feature nicht." + } } } diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index 6319c5012e..062b59d38b 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -89,14 +89,29 @@ } }, "errors": { - "context": "context", + "context": "Context", "errorCode": "Error Code", "transactionId": "Transaction ID", "moreInfo": "For more information, see", - "AGR7UzkhA1": "Not found", - "FtR7UznKU1": "Already exists", - "9BR7qpDAe1": "Password change not allowed", - "2wR7UzpPG1": "Concurrent modifications", - "9SR8G0kmU1": "Feature not supported" + "AGR7UzkhA1": { + "displayName": "Not found", + "description": "The requested entity could not be found. It may have been deleted in another session." + }, + "FtR7UznKU1": { + "displayName": "Already exists", + "description": "There is already an entity with the same key values." + }, + "9BR7qpDAe1": { + "displayName": "Password change not allowed", + "description": "You do not have the permission to change the password." + }, + "2wR7UzpPG1": { + "displayName": "Concurrent modifications", + "description": "The entity has been modified concurrently by another user or another process. Please reload the entity." + }, + "9SR8G0kmU1": { + "displayName": "Feature not supported", + "description": "The version control system for this repository does not support the requested feature." + } } } From 8e9df1ce34963a646f111d1e7f5a4991d6dff487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= <rene.pfeuffer@cloudogu.com> Date: Fri, 1 Mar 2019 15:43:33 +0100 Subject: [PATCH 10/18] Add more error messages --- .../main/resources/locales/de/plugins.json | 24 +++++++++++++++++++ .../main/resources/locales/en/plugins.json | 24 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index 2f619f0b8a..3a1ceb8d5b 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -112,6 +112,30 @@ "9SR8G0kmU1": { "displayName": "Feature nicht unterstützt", "description": "Das Versionsverwaltungssystem dieses Repo unterstützt das angefragte Feature nicht." + }, + "CmR8GCJb31": { + "displayName": "Interner Serverfehler", + "description": "Im Server ist ein interner Fehler aufgetreten. Bitte wenden Sie sich an ihren Administrator für weitere Hinweise." + }, + "92RCCCMHO1": { + "displayName": "Eine interne URL wurde nicht gefunden", + "description": "Ein interner Serveraufruf konnte nicht verarbeitet werden. Bitte wenden Sie sich an ihren Administrator für weitere Hinweise." + }, + "2VRCrvpL71": { + "displayName": "Ungültiges Datenformat", + "description": "Die zum Server gesendeten Daten konnten nicht verarbeitet werden. Bitte prüfen Sie die eingegebenen Werte oder wenden Sie sich an ihren Administrator für weitere Hinweise." + }, + "8pRBYDURx1": { + "displayName": "Ungültiger Datentyp", + "description": "Die zum Server gesendeten Daten hatten einen ungültigen Typen. Bitte wenden Sie sich an ihren Administrator für weitere Hinweise." + }, + "1wR7ZBe7H1": { + "displayName": "Ungültige Eingabe", + "description": "Die eingegebenen Daten konnten nicht validiert werden. Bitte korrigieren Sie die Eingaben und senden Sie sie erneut." + }, + "3zR9vPNIE1": { + "displayName": "Ungültige Eingabe", + "description": "Die eingegebenen Daten konnten nicht validiert werden. Bitte korrigieren Sie die Eingaben und senden Sie sie erneut." } } } diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index 062b59d38b..fa28b028e9 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -112,6 +112,30 @@ "9SR8G0kmU1": { "displayName": "Feature not supported", "description": "The version control system for this repository does not support the requested feature." + }, + "CmR8GCJb31": { + "displayName": "Internal server error", + "description": "The server encountered an internal error. Please contact your administrator for further assistance." + }, + "92RCCCMHO1": { + "displayName": "An internal URL could not be found", + "description": "An internal request could not be handled by the server. Please contact your administrator for further assistance." + }, + "2VRCrvpL71": { + "displayName": "Illegal data format", + "description": "The data sent to the server could not be handled. Please check the values you have entered or contact your administrator for further assistance." + }, + "8pRBYDURx1": { + "displayName": "Illegal data type", + "description": "The data sent to the server had an illegal data type. Please contact your administrator for further assistance." + }, + "1wR7ZBe7H1": { + "displayName": "Illegal input", + "description": "The values could not be validated. Please correct your input and try again." + }, + "3zR9vPNIE1": { + "displayName": "Illegal input", + "description": "The values could not be validated. Please correct your input and try again." } } } From dadfe4a8afca15b78a9bf81c62bc83df288e05f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= <rene.pfeuffer@cloudogu.com> Date: Mon, 4 Mar 2019 08:44:41 +0100 Subject: [PATCH 11/18] Fix media type for validation error --- .../src/BackendErrorNotification.js | 33 ++++++++++++------- .../v2/ResteasyValidationExceptionMapper.java | 4 +-- ...cmConstraintValidationExceptionMapper.java | 4 +-- 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js index 0995417e57..4d36495dd3 100644 --- a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js +++ b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js @@ -46,18 +46,7 @@ class BackendErrorNotification extends React.Component<Props> { const { error, t } = this.props; return ( <> - <p> - <strong>{t("errors.context")}</strong> - </p> - <ul> - {error.context.map((context, index) => { - return ( - <li key={index}> - <strong>{context.type}:</strong> {context.id} - </li> - ); - })} - </ul> + {this.renderContext(error)} {this.renderMoreInformationLink(error)} <div className="level is-size-7"> <div className="left"> @@ -71,6 +60,26 @@ class BackendErrorNotification extends React.Component<Props> { ); }; + renderContext = (error: BackendError) => { + if (error.context) { + return <> + <p> + <strong>{t("errors.context")}</strong> + </p> + <ul> + {error.context.map((context, index) => { + return ( + <li key={index}> + <strong>{context.type}:</strong> {context.id} + </li> + ); + })} + </ul> + </>; + } + }; + + renderMoreInformationLink = (error: BackendError) => { const { t } = this.props; if (error.url) { diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/ResteasyValidationExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/ResteasyValidationExceptionMapper.java index 63582e10b8..0b8b44f308 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/ResteasyValidationExceptionMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/ResteasyValidationExceptionMapper.java @@ -2,9 +2,9 @@ package sonia.scm.api.v2; import org.jboss.resteasy.api.validation.ResteasyViolationException; import sonia.scm.api.v2.resources.ResteasyViolationExceptionToErrorDtoMapper; +import sonia.scm.web.VndMediaType; import javax.inject.Inject; -import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; @@ -23,7 +23,7 @@ public class ResteasyValidationExceptionMapper implements ExceptionMapper<Restea public Response toResponse(ResteasyViolationException exception) { return Response .status(Response.Status.BAD_REQUEST) - .type(MediaType.APPLICATION_JSON_TYPE) + .type(VndMediaType.ERROR_TYPE) .entity(mapper.map(exception)) .build(); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/ScmConstraintValidationExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/ScmConstraintValidationExceptionMapper.java index 991aeedaeb..17883e41cb 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/ScmConstraintValidationExceptionMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/ScmConstraintValidationExceptionMapper.java @@ -2,9 +2,9 @@ package sonia.scm.api.v2; import sonia.scm.ScmConstraintViolationException; import sonia.scm.api.v2.resources.ScmViolationExceptionToErrorDtoMapper; +import sonia.scm.web.VndMediaType; import javax.inject.Inject; -import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; @@ -23,7 +23,7 @@ public class ScmConstraintValidationExceptionMapper implements ExceptionMapper<S public Response toResponse(ScmConstraintViolationException exception) { return Response .status(Response.Status.BAD_REQUEST) - .type(MediaType.APPLICATION_JSON_TYPE) + .type(VndMediaType.ERROR_TYPE) .entity(mapper.map(exception)) .build(); } From adf92db9b24b3099d548133025e9642915badf5d Mon Sep 17 00:00:00 2001 From: Philipp Czora <philipp.czora@cloudogu.com> Date: Mon, 4 Mar 2019 11:09:12 +0100 Subject: [PATCH 12/18] Also render violations (if present) --- .../src/BackendErrorNotification.js | 54 +++++++++++++------ .../packages/ui-components/src/errors.js | 11 +++- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js index 4d36495dd3..63eb36c130 100644 --- a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js +++ b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js @@ -18,6 +18,7 @@ class BackendErrorNotification extends React.Component<Props> { <div className="content"> <p className="subtitle">{this.renderErrorName()}</p> <p>{this.renderErrorDescription()}</p> + <p>{this.renderViolations()}</p> {this.renderMetadata()} </div> </Notification> @@ -42,6 +43,28 @@ class BackendErrorNotification extends React.Component<Props> { return translation; }; + renderViolations = () => { + const { error, t } = this.props; + if (error.violations) { + return ( + <> + <p> + <strong>{t("errors.violations")}</strong> + </p> + <ul> + {error.violations.map((violation, index) => { + return ( + <li key={index}> + <strong>{violation.path}:</strong> {violation.message} + </li> + ); + })} + </ul> + </> + ); + } + }; + renderMetadata = () => { const { error, t } = this.props; return ( @@ -62,24 +85,25 @@ class BackendErrorNotification extends React.Component<Props> { renderContext = (error: BackendError) => { if (error.context) { - return <> - <p> - <strong>{t("errors.context")}</strong> - </p> - <ul> - {error.context.map((context, index) => { - return ( - <li key={index}> - <strong>{context.type}:</strong> {context.id} - </li> - ); - })} - </ul> - </>; + return ( + <> + <p> + <strong>{t("errors.context")}</strong> + </p> + <ul> + {error.context.map((context, index) => { + return ( + <li key={index}> + <strong>{context.type}:</strong> {context.id} + </li> + ); + })} + </ul> + </> + ); } }; - renderMoreInformationLink = (error: BackendError) => { const { t } = this.props; if (error.url) { diff --git a/scm-ui-components/packages/ui-components/src/errors.js b/scm-ui-components/packages/ui-components/src/errors.js index 9b278c7ddd..de425109b4 100644 --- a/scm-ui-components/packages/ui-components/src/errors.js +++ b/scm-ui-components/packages/ui-components/src/errors.js @@ -1,12 +1,14 @@ // @flow type Context = { type: string, id: string }[]; +type Violation = { path: string, message: string }; export type BackendErrorContent = { transactionId: string, errorCode: string, message: string, url?: string, - context: Context + context: Context, + violations: Violation[] }; export class BackendError extends Error { @@ -15,6 +17,7 @@ export class BackendError extends Error { url: ?string; context: Context = []; statusCode: number; + violations: Violation[]; constructor(content: BackendErrorContent, name: string, statusCode: number) { super(content.message); @@ -24,6 +27,7 @@ export class BackendError extends Error { this.url = content.url; this.context = content.context; this.statusCode = statusCode; + this.violations = content.violations; } } @@ -53,5 +57,8 @@ export function createBackendError( } export function isBackendError(response: Response) { - return response.headers.get("Content-Type") === "application/vnd.scmm-error+json;v=2"; + return ( + response.headers.get("Content-Type") === + "application/vnd.scmm-error+json;v=2" + ); } From f68398e599b9e4c04732b69d9f6c765d1ba1d2b4 Mon Sep 17 00:00:00 2001 From: Philipp Czora <philipp.czora@cloudogu.com> Date: Mon, 4 Mar 2019 11:19:33 +0100 Subject: [PATCH 13/18] Minor refactoring --- .../ui-components/src/BackendErrorNotification.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js index 63eb36c130..31106593c4 100644 --- a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js +++ b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js @@ -69,8 +69,8 @@ class BackendErrorNotification extends React.Component<Props> { const { error, t } = this.props; return ( <> - {this.renderContext(error)} - {this.renderMoreInformationLink(error)} + {this.renderContext()} + {this.renderMoreInformationLink()} <div className="level is-size-7"> <div className="left"> {t("errors.transactionId")} {error.transactionId} @@ -83,7 +83,8 @@ class BackendErrorNotification extends React.Component<Props> { ); }; - renderContext = (error: BackendError) => { + renderContext = () => { + const { error, t} = this.props; if (error.context) { return ( <> @@ -104,8 +105,8 @@ class BackendErrorNotification extends React.Component<Props> { } }; - renderMoreInformationLink = (error: BackendError) => { - const { t } = this.props; + renderMoreInformationLink = () => { + const { error, t } = this.props; if (error.url) { return ( <p> From 6c06352de2a57918ac6ff15d179e9308aef4d6ea Mon Sep 17 00:00:00 2001 From: Philipp Czora <philipp.czora@cloudogu.com> Date: Mon, 4 Mar 2019 11:49:12 +0100 Subject: [PATCH 14/18] Added ui error handling for 403 errors --- .../packages/ui-components/src/ErrorNotification.js | 12 ++++++++++-- .../packages/ui-components/src/ErrorPage.js | 4 ++-- .../packages/ui-components/src/apiclient.js | 5 ++++- .../packages/ui-components/src/errors.js | 8 ++++++++ scm-ui/public/locales/de/commons.json | 3 ++- scm-ui/public/locales/en/commons.json | 3 ++- 6 files changed, 28 insertions(+), 7 deletions(-) diff --git a/scm-ui-components/packages/ui-components/src/ErrorNotification.js b/scm-ui-components/packages/ui-components/src/ErrorNotification.js index 380ea04082..b8acf89733 100644 --- a/scm-ui-components/packages/ui-components/src/ErrorNotification.js +++ b/scm-ui-components/packages/ui-components/src/ErrorNotification.js @@ -1,7 +1,7 @@ //@flow import React from "react"; import { translate } from "react-i18next"; -import { BackendError, UnauthorizedError } from "./errors"; +import { BackendError, ForbiddenError, UnauthorizedError } from "./errors"; import Notification from "./Notification"; import BackendErrorNotification from "./BackendErrorNotification"; @@ -27,7 +27,15 @@ class ErrorNotification extends React.Component<Props> { </a> </Notification> ); - } else { + } else if (error instanceof ForbiddenError) { + return ( + <Notification type="danger"> + <strong>{t("error-notification.prefix")}:</strong>{" "} + {t("error-notification.forbidden")} + </Notification> + ) + } else + { return ( <Notification type="danger"> <strong>{t("error-notification.prefix")}:</strong> {error.message} diff --git a/scm-ui-components/packages/ui-components/src/ErrorPage.js b/scm-ui-components/packages/ui-components/src/ErrorPage.js index 8947e702c8..b86f374263 100644 --- a/scm-ui-components/packages/ui-components/src/ErrorPage.js +++ b/scm-ui-components/packages/ui-components/src/ErrorPage.js @@ -1,7 +1,7 @@ //@flow import React from "react"; import ErrorNotification from "./ErrorNotification"; -import { BackendError } from "./errors"; +import { BackendError, ForbiddenError } from "./errors"; type Props = { error: Error, @@ -26,7 +26,7 @@ class ErrorPage extends React.Component<Props> { renderSubtitle = () => { const { error, subtitle } = this.props; - if (error instanceof BackendError) { + if (error instanceof BackendError || error instanceof ForbiddenError) { return null; } return <p className="subtitle">{subtitle}</p> diff --git a/scm-ui-components/packages/ui-components/src/apiclient.js b/scm-ui-components/packages/ui-components/src/apiclient.js index ce7de1b757..77ebe8cf4e 100644 --- a/scm-ui-components/packages/ui-components/src/apiclient.js +++ b/scm-ui-components/packages/ui-components/src/apiclient.js @@ -1,6 +1,6 @@ // @flow import { contextPath } from "./urls"; -import {createBackendError, isBackendError, UnauthorizedError} from "./errors"; +import { createBackendError, ForbiddenError, isBackendError, UnauthorizedError } from "./errors"; import type { BackendErrorContent } from "./errors"; const fetchOptions: RequestOptions = { @@ -22,7 +22,10 @@ function handleFailure(response: Response) { } else { if (response.status === 401) { throw new UnauthorizedError("Unauthorized", 401); + } else if (response.status === 403) { + throw new ForbiddenError("Forbidden", 403); } + throw new Error("server returned status code " + response.status); } } diff --git a/scm-ui-components/packages/ui-components/src/errors.js b/scm-ui-components/packages/ui-components/src/errors.js index de425109b4..13e4996cf7 100644 --- a/scm-ui-components/packages/ui-components/src/errors.js +++ b/scm-ui-components/packages/ui-components/src/errors.js @@ -39,6 +39,14 @@ export class UnauthorizedError extends Error { } } +export class ForbiddenError extends Error { + statusCode: number; + constructor(message: string, statusCode: number) { + super(message); + this.statusCode = statusCode; + } +} + export class NotFoundError extends BackendError { constructor(content: BackendErrorContent, statusCode: number) { super(content, "NotFoundError", statusCode); diff --git a/scm-ui/public/locales/de/commons.json b/scm-ui/public/locales/de/commons.json index f32d764ce8..f8007e1b51 100644 --- a/scm-ui/public/locales/de/commons.json +++ b/scm-ui/public/locales/de/commons.json @@ -23,7 +23,8 @@ "prefix": "Fehler", "loginLink": "Erneute Anmeldung", "timeout": "Die Session ist abgelaufen.", - "wrong-login-credentials": "Ungültige Anmeldedaten" + "wrong-login-credentials": "Ungültige Anmeldedaten", + "forbidden": "Sie haben nicht die Berechtigung, diese Entität zu sehen" }, "loading": { "alt": "Lade ..." diff --git a/scm-ui/public/locales/en/commons.json b/scm-ui/public/locales/en/commons.json index fed749a200..cc4edde82c 100644 --- a/scm-ui/public/locales/en/commons.json +++ b/scm-ui/public/locales/en/commons.json @@ -23,7 +23,8 @@ "prefix": "Error", "loginLink": "You can login here again.", "timeout": "The session has expired", - "wrong-login-credentials": "Invalid credentials" + "wrong-login-credentials": "Invalid credentials", + "forbidden": "You don't have permission to view this entity" }, "loading": { "alt": "Loading ..." From 768915c75aa52f66b1dc9052822cc1b7660c0b3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= <rene.pfeuffer@cloudogu.com> Date: Mon, 4 Mar 2019 14:09:48 +0100 Subject: [PATCH 15/18] Do not map not found exception manually --- .../java/sonia/scm/api/v2/resources/BranchRootResource.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BranchRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BranchRootResource.java index 71b1127ad8..d19c1a9e03 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BranchRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BranchRootResource.java @@ -80,8 +80,6 @@ public class BranchRootResource { .build(); } catch (CommandNotSupportedException ex) { return Response.status(Response.Status.BAD_REQUEST).build(); - } catch (NotFoundException e) { - return Response.status(Response.Status.NOT_FOUND).build(); } } From 03aa639aa18c31562957a138442f72f88cf457a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= <rene.pfeuffer@cloudogu.com> Date: Mon, 4 Mar 2019 15:54:10 +0100 Subject: [PATCH 16/18] Clarify german texts --- scm-ui/public/locales/de/commons.json | 2 +- scm-webapp/src/main/resources/locales/de/plugins.json | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/scm-ui/public/locales/de/commons.json b/scm-ui/public/locales/de/commons.json index f8007e1b51..51a493fc40 100644 --- a/scm-ui/public/locales/de/commons.json +++ b/scm-ui/public/locales/de/commons.json @@ -24,7 +24,7 @@ "loginLink": "Erneute Anmeldung", "timeout": "Die Session ist abgelaufen.", "wrong-login-credentials": "Ungültige Anmeldedaten", - "forbidden": "Sie haben nicht die Berechtigung, diese Entität zu sehen" + "forbidden": "Sie haben nicht die Berechtigung, diesen Datensatz zu sehen" }, "loading": { "alt": "Lade ..." diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index 3a1ceb8d5b..ae99233510 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -95,11 +95,11 @@ "moreInfo": "Für mehr Informationen, siehe", "AGR7UzkhA1": { "displayName": "Nicht gefunden", - "description": "Die angefragte Entität konnte nicht gefunden werden. Möglicherweise wurde es in einer weiteren Session gelöscht." + "description": "Der gewünschte Datensatz konnte nicht gefunden werden. Möglicherweise wurde er in einer weiteren Session gelöscht." }, "FtR7UznKU1": { "displayName": "Existiert bereits", - "description": "Eine Entität mit den gegebenen Schlüsselwerten existiert bereits" + "description": "Ein Datensatz mit den gegebenen Schlüsselwerten existiert bereits" }, "9BR7qpDAe1": { "displayName": "Passwortänderung nicht erlaubt", @@ -107,11 +107,11 @@ }, "2wR7UzpPG1": { "displayName": "Konkurrierende Änderungen", - "description": "Die Entität wurde konkurrierend von einem anderen Benutzer oder einem anderen Prozess modifiziert. Bitte laden sie die Entität erneut." + "description": "Der Datensatz wurde konkurrierend von einem anderen Benutzer oder einem anderen Prozess modifiziert. Bitte laden sie die Daten erneut." }, "9SR8G0kmU1": { "displayName": "Feature nicht unterstützt", - "description": "Das Versionsverwaltungssystem dieses Repo unterstützt das angefragte Feature nicht." + "description": "Das Versionsverwaltungssystem dieses Repositories unterstützt das angefragte Feature nicht." }, "CmR8GCJb31": { "displayName": "Interner Serverfehler", From eca982e796b12c5a33d90386297ef80629bc009f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= <rene.pfeuffer@cloudogu.com> Date: Mon, 4 Mar 2019 16:47:44 +0100 Subject: [PATCH 17/18] fIX config error if link is missing --- scm-ui/public/locales/de/config.json | 3 ++- scm-ui/public/locales/en/config.json | 3 ++- .../src/config/components/form/ConfigForm.js | 19 +++++++++++++++++-- scm-ui/src/config/containers/GlobalConfig.js | 16 ++++++++++------ 4 files changed, 31 insertions(+), 10 deletions(-) diff --git a/scm-ui/public/locales/de/config.json b/scm-ui/public/locales/de/config.json index 5767a2b376..b67bf90262 100644 --- a/scm-ui/public/locales/de/config.json +++ b/scm-ui/public/locales/de/config.json @@ -9,7 +9,8 @@ "config-form": { "submit": "Speichern", "submit-success-notification": "Einstellungen wurden erfolgreich geändert!", - "no-permission-notification": "Hinweis: Es fehlen Berechtigungen zum Bearbeiten der Einstellungen!" + "no-read-permission-notification": "Hinweis: Es fehlen Berechtigungen zum Lesen der Einstellungen!", + "no-write-permission-notification": "Hinweis: Es fehlen Berechtigungen zum Bearbeiten der Einstellungen!" }, "proxy-settings": { "name": "Proxy Einstellungen", diff --git a/scm-ui/public/locales/en/config.json b/scm-ui/public/locales/en/config.json index b08c5c2d1b..1b42878015 100644 --- a/scm-ui/public/locales/en/config.json +++ b/scm-ui/public/locales/en/config.json @@ -9,7 +9,8 @@ "config-form": { "submit": "Submit", "submit-success-notification": "Configuration changed successfully!", - "no-permission-notification": "Please note: You do not have the permission to edit the config!" + "no-read-permission-notification": "Please note: You do not have the permission to see the config!", + "no-write-permission-notification": "Please note: You do not have the permission to edit the config!" }, "proxy-settings": { "name": "Proxy Settings", diff --git a/scm-ui/src/config/components/form/ConfigForm.js b/scm-ui/src/config/components/form/ConfigForm.js index dc3f20c95d..7b650ccbfd 100644 --- a/scm-ui/src/config/components/form/ConfigForm.js +++ b/scm-ui/src/config/components/form/ConfigForm.js @@ -14,6 +14,7 @@ type Props = { config?: Config, loading?: boolean, t: string => string, + configReadPermission: boolean, configUpdatePermission: boolean }; @@ -84,16 +85,30 @@ class ConfigForm extends React.Component<Props, State> { }; render() { - const { loading, t, configUpdatePermission } = this.props; + const { + loading, + t, + configReadPermission, + configUpdatePermission + } = this.props; const config = this.state.config; let noPermissionNotification = null; + if (!configReadPermission) { + return ( + <Notification + type={"danger"} + children={t("config-form.no-read-permission-notification")} + /> + ); + } + if (this.state.showNotification) { noPermissionNotification = ( <Notification type={"info"} - children={t("config-form.no-permission-notification")} + children={t("config-form.no-write-permission-notification")} onClose={() => this.onClose()} /> ); diff --git a/scm-ui/src/config/containers/GlobalConfig.js b/scm-ui/src/config/containers/GlobalConfig.js index de01e98aab..fd3ee04098 100644 --- a/scm-ui/src/config/containers/GlobalConfig.js +++ b/scm-ui/src/config/containers/GlobalConfig.js @@ -1,11 +1,7 @@ // @flow import React from "react"; import { translate } from "react-i18next"; -import { - Title, - Loading, - ErrorNotification -} from "@scm-manager/ui-components"; +import { Title, Loading, ErrorNotification } from "@scm-manager/ui-components"; import { fetchConfig, getFetchConfigFailure, @@ -39,6 +35,7 @@ type Props = { }; type State = { + configReadPermission: boolean, configChanged: boolean }; @@ -47,13 +44,18 @@ class GlobalConfig extends React.Component<Props, State> { super(props); this.state = { + configReadPermission: true, configChanged: false }; } componentDidMount() { this.props.configReset(); - this.props.fetchConfig(this.props.configLink); + if (this.props.configLink) { + this.props.fetchConfig(this.props.configLink); + } else { + this.setState({configReadPermission: false}); + } } modifyConfig = (config: Config) => { @@ -102,6 +104,7 @@ class GlobalConfig extends React.Component<Props, State> { renderContent = () => { const { error, loading, config, configUpdatePermission } = this.props; + const { configReadPermission } = this.state; if (!error) { return ( <> @@ -111,6 +114,7 @@ class GlobalConfig extends React.Component<Props, State> { config={config} loading={loading} configUpdatePermission={configUpdatePermission} + configReadPermission={configReadPermission} /> </> ); From 3b4af52af917cf111b0e9586b232787ff1b74c1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= <rene.pfeuffer@cloudogu.com> Date: Mon, 4 Mar 2019 16:12:30 +0000 Subject: [PATCH 18/18] Close branch feature/ui-error-handling