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

@@ -0,0 +1,2 @@
+- type: fixed
+  description: users cannot be edited ([#2084](https://github.com/scm-manager/scm-manager/pull/2084))

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");
+  });
+});

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"
   }
-}
+}

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;

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(<I18nextProvider i18n={i18nTest}>{component}</I18nextProvider>);
+
+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(<UserForm submitForm={mockSubmitForm} />);
+
+    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(<UserForm submitForm={mockSubmitForm} />);
+
+    fireEvent.click(screen.getByTestId("submit-button"));
+
+    expect(mockSubmitForm).not.toBeCalled();
+  });
+
+  it("should prevent to submit form without user id", () => {
+    const mockSubmitForm = jest.fn();
+
+    renderWithI18n(<UserForm submitForm={mockSubmitForm} />);
+
+    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(<UserForm submitForm={mockSubmitForm} />);
+
+    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(<UserForm submitForm={mockSubmitForm} />);
+
+    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(<UserForm submitForm={mockSubmitForm} />);
+
+    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(<UserForm user={user} submitForm={mockSubmitForm} />);
+
+    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(<UserForm user={user} submitForm={mockSubmitForm} />);
+
+    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(<UserForm user={user} submitForm={mockSubmitForm} />);
+
+    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(<UserForm user={user} submitForm={mockSubmitForm} />);
+
+    fireEvent.click(screen.getByTestId("submit-button"));
+
+    expect(mockSubmitForm).not.toBeCalled();
+  });
+
+  it("should prevent to edit user with incorrect email", () => {
+    const mockSubmitForm = jest.fn();
+
+    renderWithI18n(<UserForm user={user} submitForm={mockSubmitForm} />);
+
+    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(<UserForm user={user} submitForm={mockSubmitForm} />);
+
+    fireEvent.change(screen.getByTestId("input-displayname"), {
+      target: { value: "" },
+    });
+
+    fireEvent.click(screen.getByTestId("submit-button"));
+
+    expect(mockSubmitForm).not.toBeCalled();
+  });
+});

scm-ui/ui-webapp/src/users/components/UserForm.tsx

@@ -91,7 +91,7 @@ const UserForm: FC<Props> = ({ 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<Props> = ({ submitForm, user, loading }) => {
                   onChange={(active) => setUserState({ ...userState, active })}
                   checked={userState ? userState.active : false}
                   helpText={t("help.activeHelpText")}
+                  testId="checkbox-active"
                 />
               </div>
             </div>

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. */

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"