From fc2a40cab51dc76a6d81f05ddc1e8e719caaf14d Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Tue, 12 Jul 2022 10:45:05 +0200 Subject: [PATCH] Fix users not being editable (#2084) Since 2.34.0, users could not be edited. This pull request fixes the issue and adds unit tests. --- gradle/changelog/fix_user_edit.yaml | 2 + .../cypress/integration/edit_user_spec.ts | 57 +++++ scm-ui/ui-webapp/package.json | 3 +- scm-ui/ui-webapp/src/i18n.mock.ts | 47 ++++ .../src/users/components/UserForm.test.tsx | 213 ++++++++++++++++++ .../src/users/components/UserForm.tsx | 3 +- scm-ui/ui-webapp/tsconfig.json | 1 + yarn.lock | 73 ++++++ 8 files changed, 397 insertions(+), 2 deletions(-) create mode 100644 gradle/changelog/fix_user_edit.yaml create mode 100644 scm-ui/e2e-tests/cypress/integration/edit_user_spec.ts create mode 100644 scm-ui/ui-webapp/src/i18n.mock.ts create mode 100644 scm-ui/ui-webapp/src/users/components/UserForm.test.tsx diff --git a/gradle/changelog/fix_user_edit.yaml b/gradle/changelog/fix_user_edit.yaml new file mode 100644 index 0000000000..6c05b280f7 --- /dev/null +++ b/gradle/changelog/fix_user_edit.yaml @@ -0,0 +1,2 @@ +- type: fixed + description: users cannot be edited ([#2084](https://github.com/scm-manager/scm-manager/pull/2084)) diff --git a/scm-ui/e2e-tests/cypress/integration/edit_user_spec.ts b/scm-ui/e2e-tests/cypress/integration/edit_user_spec.ts new file mode 100644 index 0000000000..d1002ccd2c --- /dev/null +++ b/scm-ui/e2e-tests/cypress/integration/edit_user_spec.ts @@ -0,0 +1,57 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { hri } from "human-readable-ids"; + +describe("Edit User", () => { + + let userNameToEdit: string; + + beforeEach(() => { + // Create user and login + const username = hri.random(); + const password = hri.random(); + cy.restCreateUser(username, password); + cy.restLogin(username, password); + cy.restSetUserPermissions(username, ["user:*"]); + + userNameToEdit = hri.random(); + cy.restCreateUser(userNameToEdit, hri.random()); + }); + + it("should edit user", () => { + // Prepare data + const newUser = hri.random(); + const newEmail = `${hri.random()}@${hri.random()}.com`; + + // Act + cy.visit(`/user/${userNameToEdit}/settings/general`); + cy.byTestId("input-displayname").clear().type(newUser); + cy.byTestId("input-mail").clear().type(newEmail); + cy.byTestId("submit-button").click(); + + // Assert + cy.get("div.notification.is-success").should("exist"); + }); +}); diff --git a/scm-ui/ui-webapp/package.json b/scm-ui/ui-webapp/package.json index 2af7868542..542cb5b517 100644 --- a/scm-ui/ui-webapp/package.json +++ b/scm-ui/ui-webapp/package.json @@ -38,6 +38,7 @@ "@scm-manager/eslint-config": "^2.15.1", "@scm-manager/jest-preset": "^2.13.0", "@scm-manager/ui-tests": "2.37.2-SNAPSHOT", + "@testing-library/react": "^12.1.5", "@types/classnames": "^2.2.9", "@types/enzyme": "^3.10.3", "@types/fetch-mock": "^7.3.1", @@ -65,4 +66,4 @@ "publishConfig": { "access": "public" } -} \ No newline at end of file +} diff --git a/scm-ui/ui-webapp/src/i18n.mock.ts b/scm-ui/ui-webapp/src/i18n.mock.ts new file mode 100644 index 0000000000..b16df09ff9 --- /dev/null +++ b/scm-ui/ui-webapp/src/i18n.mock.ts @@ -0,0 +1,47 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import i18n from "i18next"; +import { initReactI18next } from "react-i18next"; +import users from "../public/locales/en/users.json"; +import commons from "../public/locales/en/commons.json"; + +i18n.use(initReactI18next).init({ + lng: "en", + fallbackLng: "en", + + // have a common namespace used around the full app + ns: ["translationsNS"], + defaultNS: "translationsNS", + + debug: true, + + interpolation: { + escapeValue: false, // not needed for react!! + }, + + resources: { en: { users, commons } }, +}); + +export default i18n; diff --git a/scm-ui/ui-webapp/src/users/components/UserForm.test.tsx b/scm-ui/ui-webapp/src/users/components/UserForm.test.tsx new file mode 100644 index 0000000000..2e5f84bf5e --- /dev/null +++ b/scm-ui/ui-webapp/src/users/components/UserForm.test.tsx @@ -0,0 +1,213 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import * as React from "react"; +import { fireEvent, render, screen } from "@testing-library/react"; + +import UserForm from "./UserForm"; +import { I18nextProvider } from "react-i18next"; +import i18nTest from "../../i18n.mock"; +import { User } from "@scm-manager/ui-types"; + +const renderWithI18n = (component) => render({component}); + +describe("for user creation", () => { + const fillForm = (userId: string, displayName: string, password: string, confirmation: string) => { + fireEvent.change(screen.getByTestId("input-username"), { + target: { value: userId }, + }); + + fireEvent.change(screen.getByTestId("input-displayname"), { + target: { value: displayName }, + }); + + fireEvent.change(screen.getByTestId("input-password"), { + target: { value: password }, + }); + + fireEvent.change(screen.getByTestId("input-password-confirmation"), { + target: { value: confirmation }, + }); + }; + + it("should allow to create user", () => { + const mockSubmitForm = jest.fn(); + + renderWithI18n(); + + fillForm("trillian", "Tricia McMillan", "password", "password"); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(mockSubmitForm).toBeCalled(); + }); + + it("should prevent to submit empty form", () => { + const mockSubmitForm = jest.fn(); + + renderWithI18n(); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(mockSubmitForm).not.toBeCalled(); + }); + + it("should prevent to submit form without user id", () => { + const mockSubmitForm = jest.fn(); + + renderWithI18n(); + + fillForm("", "Arthur Dent", "password", "password"); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(mockSubmitForm).not.toBeCalled(); + }); + + it("should prevent to submit form without display name", () => { + const mockSubmitForm = jest.fn(); + + renderWithI18n(); + + fillForm("trillian", "", "password", "password"); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(mockSubmitForm).not.toBeCalled(); + }); + + it("should prevent to submit form without password", () => { + const mockSubmitForm = jest.fn(); + + renderWithI18n(); + + fillForm("trillian", "Tricia McMillan", "", ""); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(mockSubmitForm).not.toBeCalled(); + }); + + it("should prevent to submit form with wrong password confirmation", () => { + const mockSubmitForm = jest.fn(); + + renderWithI18n(); + + fillForm("trillian", "Tricia McMillan", "password", "different"); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(mockSubmitForm).not.toBeCalled(); + }); +}); + +describe("for user edit", () => { + const user: User = { + name: "trillian", + mail: "tricia@hog.space", + displayName: "Tricia McMillan", + password: undefined, + active: true, + external: false, + _links: {}, + }; + + it("should allow to edit user with changed display name", () => { + const mockSubmitForm = jest.fn(); + + renderWithI18n(); + + fireEvent.change(screen.getByTestId("input-displayname"), { + target: { value: "Just Tricia" }, + }); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(mockSubmitForm).toBeCalled(); + }); + + it("should allow to edit user with changed email", () => { + const mockSubmitForm = jest.fn(); + + renderWithI18n(); + + fireEvent.change(screen.getByTestId("input-mail"), { + target: { value: "tricia@hg2g.com" }, + }); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(mockSubmitForm).toBeCalled(); + }); + + it("should allow to edit user with changed active flag", () => { + const mockSubmitForm = jest.fn(); + + renderWithI18n(); + + fireEvent.click(screen.getByTestId("checkbox-active")); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(mockSubmitForm).toBeCalled(); + }); + + it("should prevent to submit unchanged user", () => { + const mockSubmitForm = jest.fn(); + + renderWithI18n(); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(mockSubmitForm).not.toBeCalled(); + }); + + it("should prevent to edit user with incorrect email", () => { + const mockSubmitForm = jest.fn(); + + renderWithI18n(); + + fireEvent.change(screen.getByTestId("input-mail"), { + target: { value: "do_not_reply" }, + }); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(mockSubmitForm).not.toBeCalled(); + }); + + it("should prevent to edit user with empty display name", () => { + const mockSubmitForm = jest.fn(); + + renderWithI18n(); + + fireEvent.change(screen.getByTestId("input-displayname"), { + target: { value: "" }, + }); + + fireEvent.click(screen.getByTestId("submit-button")); + + expect(mockSubmitForm).not.toBeCalled(); + }); +}); diff --git a/scm-ui/ui-webapp/src/users/components/UserForm.tsx b/scm-ui/ui-webapp/src/users/components/UserForm.tsx index 6808f23a2c..eadc519130 100644 --- a/scm-ui/ui-webapp/src/users/components/UserForm.tsx +++ b/scm-ui/ui-webapp/src/users/components/UserForm.tsx @@ -91,7 +91,7 @@ const UserForm: FC = ({ submitForm, user, loading }) => { mailValidationError || displayNameValidationError || nameValidationError || - (userState && !userState.external && !userState.password) || + (!user && userState && !userState.external && !userState.password) || !userState.displayName ); }; @@ -195,6 +195,7 @@ const UserForm: FC = ({ submitForm, user, loading }) => { onChange={(active) => setUserState({ ...userState, active })} checked={userState ? userState.active : false} helpText={t("help.activeHelpText")} + testId="checkbox-active" /> diff --git a/scm-ui/ui-webapp/tsconfig.json b/scm-ui/ui-webapp/tsconfig.json index ecde2a0c77..9b39855786 100644 --- a/scm-ui/ui-webapp/tsconfig.json +++ b/scm-ui/ui-webapp/tsconfig.json @@ -26,6 +26,7 @@ "strict": true, /* Enable all strict type-checking options. */ "noImplicitAny": true, /* Raise error on expressions and declarations with an implied 'any' type. */ "strictNullChecks": true, /* Enable strict null checks. */ + "resolveJsonModule": true, // "strictFunctionTypes": true, /* Enable strict checking of function types. */ // "strictBindCallApply": true, /* Enable strict 'bind', 'call', and 'apply' methods on functions. */ // "strictPropertyInitialization": true, /* Enable strict checking of property initialization in classes. */ diff --git a/yarn.lock b/yarn.lock index d46489e89b..d2f4ce18df 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4599,6 +4599,20 @@ resolve-from "^5.0.0" store2 "^2.12.0" +"@testing-library/dom@^8.0.0": + version "8.16.0" + resolved "https://registry.yarnpkg.com/@testing-library/dom/-/dom-8.16.0.tgz#d6fc50250aed17b1035ca1bd64655e342db3936a" + integrity sha512-uxF4zmnLHHDlmW4l+0WDjcgLVwCvH+OVLpD8Dfp+Bjfz85prwxWGbwXgJdLtkgjD0qfOzkJF9SmA6YZPsMYX4w== + dependencies: + "@babel/code-frame" "^7.10.4" + "@babel/runtime" "^7.12.5" + "@types/aria-query" "^4.2.0" + aria-query "^5.0.0" + chalk "^4.1.0" + dom-accessibility-api "^0.5.9" + lz-string "^1.4.4" + pretty-format "^27.0.2" + "@testing-library/react-hooks@^5.0.3": version "5.1.2" resolved "https://registry.yarnpkg.com/@testing-library/react-hooks/-/react-hooks-5.1.2.tgz#36e359d992bb652a9885c6fa9aa394639cbe8dd3" @@ -4611,6 +4625,15 @@ filter-console "^0.1.1" react-error-boundary "^3.1.0" +"@testing-library/react@^12.1.5": + version "12.1.5" + resolved "https://registry.yarnpkg.com/@testing-library/react/-/react-12.1.5.tgz#bb248f72f02a5ac9d949dea07279095fa577963b" + integrity sha512-OfTXCJUFgjd/digLUuPxa0+/3ZxsQmE7ub9kcbW/wi96Bh3o/p5vrETcBGfP17NWPGqeYYl5LTRpwyGoMC4ysg== + dependencies: + "@babel/runtime" "^7.12.5" + "@testing-library/dom" "^8.0.0" + "@types/react-dom" "<18.0.0" + "@trysound/sax@0.2.0": version "0.2.0" resolved "https://registry.yarnpkg.com/@trysound/sax/-/sax-0.2.0.tgz#cccaab758af56761eb7bf37af6f03f326dd798ad" @@ -4621,6 +4644,11 @@ resolved "https://registry.yarnpkg.com/@types/anymatch/-/anymatch-1.3.1.tgz#336badc1beecb9dacc38bea2cf32adf627a8421a" integrity sha512-/+CRPXpBDpo2RK9C68N3b2cOvO0Cf5B9aPijHsoDQTHivnGSObdOF2BRQOYjojWTDy6nQvMjmqRXIxH55VjxxA== +"@types/aria-query@^4.2.0": + version "4.2.2" + resolved "https://registry.yarnpkg.com/@types/aria-query/-/aria-query-4.2.2.tgz#ed4e0ad92306a704f9fb132a0cfcf77486dbe2bc" + integrity sha512-HnYpAE1Y6kRyKM/XkEuiRQhTHvkzMBurTHnpFLYLBGPIylZNPs9jJcuOOYWxPLJCSEtmZT0Y8rHDokKN7rRTig== + "@types/babel__code-frame@^7.0.1": version "7.0.2" resolved "https://registry.yarnpkg.com/@types/babel__code-frame/-/babel__code-frame-7.0.2.tgz#e0c0f1648cbc09a9d4e5b4ed2ae9a6f7c8f5aeb0" @@ -5008,6 +5036,13 @@ dependencies: "@types/react" "*" +"@types/react-dom@<18.0.0": + version "17.0.17" + resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-17.0.17.tgz#2e3743277a793a96a99f1bf87614598289da68a1" + integrity sha512-VjnqEmqGnasQKV0CWLevqMTXBYG9GbwuE6x3VetERLh0cq2LTptFE73MrQi2S7GkKXCf2GgwItB/melLnxfnsg== + dependencies: + "@types/react" "^17" + "@types/react-redux@5.0.7": version "5.0.7" resolved "https://registry.yarnpkg.com/@types/react-redux/-/react-redux-5.0.7.tgz#01a5181ff6757724a4b59c13852ee6c49f65c101" @@ -5086,6 +5121,15 @@ "@types/scheduler" "*" csstype "^3.0.2" +"@types/react@^17": + version "17.0.47" + resolved "https://registry.yarnpkg.com/@types/react/-/react-17.0.47.tgz#4ee71aaf4c5a9e290e03aa4d0d313c5d666b3b78" + integrity sha512-mk0BL8zBinf2ozNr3qPnlu1oyVTYq+4V7WA76RgxUAtf0Em/Wbid38KN6n4abEkvO4xMTBWmnP1FtQzgkEiJoA== + dependencies: + "@types/prop-types" "*" + "@types/scheduler" "*" + csstype "^3.0.2" + "@types/retry@^0.12.0": version "0.12.1" resolved "https://registry.yarnpkg.com/@types/retry/-/retry-0.12.1.tgz#d8f1c0d0dc23afad6dc16a9e993a0865774b4065" @@ -5915,6 +5959,11 @@ ansi-styles@^4.0.0, ansi-styles@^4.1.0: dependencies: color-convert "^2.0.1" +ansi-styles@^5.0.0: + version "5.2.0" + resolved "https://registry.yarnpkg.com/ansi-styles/-/ansi-styles-5.2.0.tgz#07449690ad45777d1924ac2abb2fc8895dba836b" + integrity sha512-Cxwpt2SfTzTtXcfOlzGEee8O+c+MmUgGrNiBcXnuWxuFJHe6a5Hz7qwhwe5OgaSYI0IJvkLqWX1ASG+cJOkEiA== + ansi-to-html@^0.6.11: version "0.6.14" resolved "https://registry.yarnpkg.com/ansi-to-html/-/ansi-to-html-0.6.14.tgz#65fe6d08bba5dd9db33f44a20aec331e0010dad8" @@ -6006,6 +6055,11 @@ aria-query@^4.2.2: "@babel/runtime" "^7.10.2" "@babel/runtime-corejs3" "^7.10.2" +aria-query@^5.0.0: + version "5.0.0" + resolved "https://registry.yarnpkg.com/aria-query/-/aria-query-5.0.0.tgz#210c21aaf469613ee8c9a62c7f86525e058db52c" + integrity sha512-V+SM7AbUwJ+EBnB8+DXs0hPZHO0W6pqBcc0dW90OwtVG02PswOu/teuARoLQjdDOH+t9pJgGnW5/Qmouf3gPJg== + arr-diff@^4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/arr-diff/-/arr-diff-4.0.0.tgz#d6461074febfec71e7e15235761a329a5dc7c520" @@ -8797,6 +8851,11 @@ doctrine@^3.0.0: dependencies: esutils "^2.0.2" +dom-accessibility-api@^0.5.9: + version "0.5.14" + resolved "https://registry.yarnpkg.com/dom-accessibility-api/-/dom-accessibility-api-0.5.14.tgz#56082f71b1dc7aac69d83c4285eef39c15d93f56" + integrity sha512-NMt+m9zFMPZe0JcY9gN224Qvk6qLIdqex29clBvc/y75ZBX9YA9wNK3frsYvu2DI1xcCIwxwnX+TlsJ2DSOADg== + dom-converter@^0.2, dom-converter@^0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/dom-converter/-/dom-converter-0.2.0.tgz#6721a9daee2e293682955b6afe416771627bb768" @@ -13789,6 +13848,11 @@ lru-cache@^6.0.0: dependencies: yallist "^4.0.0" +lz-string@^1.4.4: + version "1.4.4" + resolved "https://registry.yarnpkg.com/lz-string/-/lz-string-1.4.4.tgz#c0d8eaf36059f705796e1e344811cf4c498d3a26" + integrity sha512-0ckx7ZHRPqb0oUm8zNr+90mtf9DQB60H1wMCjBtfi62Kl3a7JbHob6gA2bC+xRvZoOL+1hzUK8jeuEIQE8svEQ== + magic-string@^0.25.7: version "0.25.9" resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.25.9.tgz#de7f9faf91ef8a1c91d02c2e5314c8277dbcdd1c" @@ -16048,6 +16112,15 @@ pretty-format@^26.0.0, pretty-format@^26.6.2: ansi-styles "^4.0.0" react-is "^17.0.1" +pretty-format@^27.0.2: + version "27.5.1" + resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-27.5.1.tgz#2181879fdea51a7a5851fb39d920faa63f01d88e" + integrity sha512-Qb1gy5OrP5+zDf2Bvnzdl3jsTf1qXVMazbvCoKhtKqVs4/YK4ozX4gKQJJVyNe+cajNPn0KoC0MC3FUmaHWEmQ== + dependencies: + ansi-regex "^5.0.1" + ansi-styles "^5.0.0" + react-is "^17.0.1" + pretty-format@^3.8.0: version "3.8.0" resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-3.8.0.tgz#bfbed56d5e9a776645f4b1ff7aa1a3ac4fa3c385"