fix review findings

2025-11-10 23:45:44 +01:00 · 2020-08-11 10:34:29 +02:00
parent a46d8c4749
commit c1cfff603b
63 changed files with 578 additions and 494 deletions
--- a/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx
+++ b/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx
@@ -122,6 +122,7 @@ class GeneralSettings extends React.Component<Props> {
                { label: t("general-settings.anonymousMode.off"), value: "OFF" }
              ]}
              helpText={t("help.allowAnonymousAccessHelpText")}
+              testId={"anonymous-mode-select"}
            />
          </div>
        </div>
--- a/scm-ui/ui-webapp/src/admin/containers/Admin.tsx
+++ b/scm-ui/ui-webapp/src/admin/containers/Admin.tsx
@@ -133,7 +133,7 @@ class Admin extends React.Component<Props> {
                  icon="fas fa-info-circle"
                  label={t("admin.menu.informationNavLink")}
                  title={t("admin.menu.informationNavLink")}
-                  className={t("admin.menu.informationNavLink")}
+                  testId="admin-information-link"
                />
                {(availablePluginsLink || installedPluginsLink) && (
                  <SubNavigation
@@ -141,13 +141,21 @@ class Admin extends React.Component<Props> {
                    icon="fas fa-puzzle-piece"
                    label={t("plugins.menu.pluginsNavLink")}
                    title={t("plugins.menu.pluginsNavLink")}
-                    className={t("plugins.menu.pluginsNavLink")}
+                    testId="admin-plugins-link"
                  >
                    {installedPluginsLink && (
-                      <NavLink to={`${url}/plugins/installed/`} label={t("plugins.menu.installedNavLink")} className={t("plugins.menu.installedNavLink")}/>
+                      <NavLink
+                        to={`${url}/plugins/installed/`}
+                        label={t("plugins.menu.installedNavLink")}
+                        testId="admin-installed-plugins-link"
+                      />
                    )}
                    {availablePluginsLink && (
-                      <NavLink to={`${url}/plugins/available/`} label={t("plugins.menu.availableNavLink")} className={t("plugins.menu.availableNavLink")} />
+                      <NavLink
+                        to={`${url}/plugins/available/`}
+                        label={t("plugins.menu.availableNavLink")}
+                        testId="admin-available-plugins-link"
+                      />
                    )}
                  </SubNavigation>
                )}
@@ -156,7 +164,7 @@ class Admin extends React.Component<Props> {
                  icon="fas fa-user-shield"
                  label={t("repositoryRole.navLink")}
                  title={t("repositoryRole.navLink")}
-                  className={t("repositoryRole.navLink")}
+                  testId="admin-repository-role-link"
                  activeWhenMatch={this.matchesRoles}
                  activeOnlyWhenExact={false}
                />
@@ -165,9 +173,13 @@ class Admin extends React.Component<Props> {
                  to={`${url}/settings/general`}
                  label={t("admin.menu.settingsNavLink")}
                  title={t("admin.menu.settingsNavLink")}
-                  className={t("admin.menu.settingsNavLink")}
+                  testId="admin-settings-link"
                >
-                  <NavLink to={`${url}/settings/general`} label={t("admin.menu.generalNavLink")} className={t("admin.menu.generalNavLink")}/>
+                  <NavLink
+                    to={`${url}/settings/general`}
+                    label={t("admin.menu.generalNavLink")}
+                    testId="admin-settings-general-link"
+                  />
                  <ExtensionPoint name="admin.setting" props={extensionProps} renderAll={true} />
                </SubNavigation>
              </SecondaryNavigation>
--- a/scm-ui/ui-webapp/src/components/LoginForm.tsx
+++ b/scm-ui/ui-webapp/src/components/LoginForm.tsx
@@ -109,18 +109,18 @@ class LoginForm extends React.Component<Props, State> {
          <ErrorNotification error={this.areCredentialsInvalid()} />
          <form onSubmit={this.handleSubmit}>
            <InputField
-              className="username"
+              testId="username-input"
              placeholder={t("login.username-placeholder")}
              autofocus={true}
              onChange={this.handleUsernameChange}
            />
            <InputField
-              className="password"
+              testId="password-input"
              placeholder={t("login.password-placeholder")}
              type="password"
              onChange={this.handlePasswordChange}
            />
-            <SubmitButton label={t("login.submit")} fullWidth={true} loading={loading} />
+            <SubmitButton label={t("login.submit")} fullWidth={true} loading={loading} testId="login-button" />
          </form>
        </TopMarginBox>
      </div>
--- a/scm-ui/ui-webapp/src/containers/Logout.tsx
+++ b/scm-ui/ui-webapp/src/containers/Logout.tsx
@@ -22,42 +22,37 @@
 * SOFTWARE.
 */
 import React from "react";
-import { connect } from "react-redux";
-import { WithTranslation, withTranslation } from "react-i18next";
+import {connect} from "react-redux";
+import {WithTranslation, withTranslation} from "react-i18next";

-import { getLogoutFailure, logout } from "../modules/auth";
-import { ErrorPage, Loading } from "@scm-manager/ui-components";
-import { getLogoutLink } from "../modules/indexResource";
-import { RouteComponentProps, withRouter } from "react-router-dom";
-import { compose } from "redux";
+import {getLogoutFailure, logout} from "../modules/auth";
+import {ErrorPage, Loading} from "@scm-manager/ui-components";
+import {getLogoutLink} from "../modules/indexResource";
+import {RouteComponentProps, withRouter} from "react-router-dom";
+import {compose} from "redux";

 type Props = RouteComponentProps &
  WithTranslation & {
-    error: Error;
-    logoutLink: string;
+  error: Error;
+  logoutLink: string;

-    // dispatcher functions
-    logout: (link: string) => void;
-  };
+  // dispatcher functions
+  logout: (link: string, callback: () => void) => void;
+};

 class Logout extends React.Component<Props> {
  componentDidMount() {
-    new Promise((resolve, reject) => {
-      setTimeout(() => {
-        if (this.props.logoutLink) {
-          this.props.logout(this.props.logoutLink);
-          resolve(this.props.history.push("/login"));
-        }
-      });
-    });
+    if (this.props.logoutLink) {
+      this.props.logout(this.props.logoutLink, () => this.props.history.push("/login"));
+    }
  }

  render() {
-    const { error, t } = this.props;
+    const {error, t} = this.props;
    if (error) {
-      return <ErrorPage title={t("logout.error.title")} subtitle={t("logout.error.subtitle")} error={error} />;
+      return <ErrorPage title={t("logout.error.title")} subtitle={t("logout.error.subtitle")} error={error}/>;
    } else {
-      return <Loading />;
+      return <Loading/>;
    }
  }
 }
@@ -73,7 +68,7 @@ const mapStateToProps = (state: any) => {

 const mapDispatchToProps = (dispatch: any) => {
  return {
-    logout: (link: string) => dispatch(logout(link))
+    logout: (link: string, callback: () => void) => dispatch(logout(link, callback))
  };
 };

--- a/scm-ui/ui-webapp/src/containers/ProfileInfo.tsx
+++ b/scm-ui/ui-webapp/src/containers/ProfileInfo.tsx
@@ -24,7 +24,13 @@
 import React from "react";
 import { WithTranslation, withTranslation } from "react-i18next";
 import { Me } from "@scm-manager/ui-types";
-import { AvatarImage, AvatarWrapper, MailLink } from "@scm-manager/ui-components";
+import {
+  AvatarImage,
+  AvatarWrapper,
+  MailLink,
+  createAttributesForTesting,
+  replaceSpacesInTestId
+} from "@scm-manager/ui-components";

 type Props = WithTranslation & {
  me: Me;
@@ -47,11 +53,11 @@ class ProfileInfo extends React.Component<Props> {
            <tbody>
              <tr>
                <th>{t("profile.username")}</th>
-                <td>{me.name}</td>
+                <td {...createAttributesForTesting(replaceSpacesInTestId(me.name))}>{me.name}</td>
              </tr>
              <tr>
                <th>{t("profile.displayName")}</th>
-                <td>{me.displayName}</td>
+                <td {...createAttributesForTesting(replaceSpacesInTestId(me.displayName))}>{me.displayName}</td>
              </tr>
              <tr>
                <th>{t("profile.mail")}</th>
--- a/scm-ui/ui-webapp/src/groups/containers/Groups.tsx
+++ b/scm-ui/ui-webapp/src/groups/containers/Groups.tsx
@@ -33,8 +33,7 @@ import {
  OverviewPageActions,
  Page,
  PageActions,
-  urls,
-  Loading
+  urls
 } from "@scm-manager/ui-components";
 import { getGroupsLink } from "../../modules/indexResource";
 import {
@@ -88,11 +87,6 @@ class Groups extends React.Component<Props> {

  render() {
    const { groups, loading, error, canAddGroups, t } = this.props;
-
-    if (loading) {
-      return <Loading />;
-    }
-
    return (
      <Page title={t("groups.title")} subtitle={t("groups.subtitle")} loading={loading || !groups} error={error}>
        {this.renderGroupTable()}
--- a/scm-ui/ui-webapp/src/modules/auth.test.ts
+++ b/scm-ui/ui-webapp/src/modules/auth.test.ts
@@ -118,7 +118,7 @@ describe("auth actions", () => {
    fetchMock.postOnce("/api/v2/auth/access_token", {
      body: {
        cookie: true,
-        grantType: "password",
+        grant_type: "password",
        username: "tricia",
        password: "secret123"
      },
--- a/scm-ui/ui-webapp/src/modules/auth.ts
+++ b/scm-ui/ui-webapp/src/modules/auth.ts
@@ -32,7 +32,8 @@ import {
  callFetchIndexResources,
  fetchIndexResources,
  fetchIndexResourcesPending,
-  fetchIndexResourcesSuccess, getLoginLink
+  fetchIndexResourcesSuccess,
+  getLoginLink
 } from "./indexResource";
 import { AnyAction } from "redux";

@@ -176,7 +177,7 @@ const callFetchMe = (link: string): Promise<Me> => {
 export const login = (loginLink: string, username: string, password: string) => {
  const loginData = {
    cookie: true,
-    grantType: "password",
+    grant_type: "password",
    username,
    password
  };
@@ -219,7 +220,7 @@ export const fetchMe = (link: string) => {
  };
 };

-export const logout = (link: string) => {
+export const logout = (link: string, callback: () => void) => {
  return function(dispatch: any) {
    dispatch(logoutPending());
    return apiClient
@@ -247,6 +248,7 @@ export const logout = (link: string) => {
          dispatch(fetchIndexResources());
        }
      })
+      .then(callback)
      .catch(error => {
        dispatch(logoutFailure(error));
      });
--- a/scm-ui/ui-webapp/src/permissions/components/PermissionCheckbox.tsx
+++ b/scm-ui/ui-webapp/src/permissions/components/PermissionCheckbox.tsx
@@ -45,6 +45,9 @@ class PermissionCheckbox extends React.Component<Props> {
      ? t("verbs.repository." + name + ".description")
      : this.translateOrDefault("permissions." + key + ".description", t("permissions.unknown"));

+    // @ts-ignore we have to use the label here because cypress gets confused with asterix and dots
+    const testId = label.replaceAll(" ", "-").toLowerCase();
+
    return (
      <Checkbox
        key={name}
@@ -54,6 +57,7 @@ class PermissionCheckbox extends React.Component<Props> {
        checked={checked}
        onChange={onChange}
        disabled={disabled}
+        testId={testId}
      />
    );
  }
--- a/scm-ui/ui-webapp/src/permissions/components/SetPermissions.tsx
+++ b/scm-ui/ui-webapp/src/permissions/components/SetPermissions.tsx
@@ -142,6 +142,7 @@ class SetPermissions extends React.Component<Props, State> {
              disabled={!this.state.permissionsChanged}
              loading={loading}
              label={t("setPermissions.button")}
+              testId="set-permissions-button"
            />
          }
        />
--- a/scm-ui/ui-webapp/src/repos/containers/Overview.tsx
+++ b/scm-ui/ui-webapp/src/repos/containers/Overview.tsx
@@ -25,7 +25,7 @@ import React from "react";
 import { connect } from "react-redux";
 import { RouteComponentProps, withRouter } from "react-router-dom";
 import { WithTranslation, withTranslation } from "react-i18next";
-import { Me, RepositoryCollection } from "@scm-manager/ui-types";
+import { RepositoryCollection } from "@scm-manager/ui-types";
 import {
  CreateButton,
  LinkPaginator,
@@ -33,8 +33,7 @@ import {
  OverviewPageActions,
  Page,
  PageActions,
-  urls,
-  Loading
+  urls
 } from "@scm-manager/ui-components";
 import { getRepositoriesLink } from "../../modules/indexResource";
 import {
@@ -45,11 +44,9 @@ import {
  isFetchReposPending
 } from "../modules/repos";
 import RepositoryList from "../components/list";
-import { fetchMe, isFetchMePending } from "../../modules/auth";

 type Props = WithTranslation &
  RouteComponentProps & {
-    me: Me;
    loading: boolean;
    error: Error;
    showCreateButton: boolean;
@@ -63,15 +60,13 @@ type Props = WithTranslation &

 class Overview extends React.Component<Props> {
  componentDidMount() {
-    const { me, fetchReposByPage, reposLink, page, location } = this.props;
-    if (me) {
-      fetchReposByPage(reposLink, page, urls.getQueryStringFromLocation(location));
-    }
+    const { fetchReposByPage, reposLink, page, location } = this.props;
+    fetchReposByPage(reposLink, page, urls.getQueryStringFromLocation(location));
  }

  componentDidUpdate = (prevProps: Props) => {
-    const { me, loading, collection, page, reposLink, location, fetchReposByPage } = this.props;
-    if (collection && page && !loading && me) {
+    const { loading, collection, page, reposLink, location, fetchReposByPage } = this.props;
+    if (collection && page && !loading) {
      const statePage: number = collection.page + 1;
      if (page !== statePage || prevProps.location.search !== location.search) {
        fetchReposByPage(reposLink, page, urls.getQueryStringFromLocation(location));
@@ -82,15 +77,16 @@ class Overview extends React.Component<Props> {
  render() {
    const { error, loading, showCreateButton, t } = this.props;

-    if (loading) {
-      return <Loading />;
-    }
-
    return (
      <Page title={t("overview.title")} subtitle={t("overview.subtitle")} loading={loading} error={error}>
        {this.renderOverview()}
        <PageActions>
-          <OverviewPageActions showCreateButton={showCreateButton} link="repos" label={t("overview.createButton")} />
+          <OverviewPageActions
+            showCreateButton={showCreateButton}
+            link="repos"
+            label={t("overview.createButton")}
+            testId="repository-overview"
+          />
        </PageActions>
      </Page>
    );
@@ -134,15 +130,13 @@ class Overview extends React.Component<Props> {

 const mapStateToProps = (state: any, ownProps: Props) => {
  const { match } = ownProps;
-  const me = fetchMe(state);
  const collection = getRepositoryCollection(state);
-  const loading = isFetchReposPending(state) || isFetchMePending(state);
+  const loading = isFetchReposPending(state);
  const error = getFetchReposFailure(state);
  const page = urls.getPageFromMatch(match);
  const showCreateButton = isAbleToCreateRepos(state);
  const reposLink = getRepositoriesLink(state);
  return {
-    me,
    collection,
    loading,
    error,
--- a/scm-ui/ui-webapp/src/users/components/navLinks/EditUserNavLink.tsx
+++ b/scm-ui/ui-webapp/src/users/components/navLinks/EditUserNavLink.tsx
@@ -42,7 +42,7 @@ class EditUserNavLink extends React.Component<Props> {
    if (!this.isEditable()) {
      return null;
    }
-    return <NavLink to={editUrl} label={t("singleUser.menu.generalNavLink")} className={t("singleUser.menu.generalNavLink")}/>;
+    return <NavLink to={editUrl} label={t("singleUser.menu.generalNavLink")} testId="user-edit-link" />;
  }
 }

--- a/scm-ui/ui-webapp/src/users/components/navLinks/SetPasswordNavLink.tsx
+++ b/scm-ui/ui-webapp/src/users/components/navLinks/SetPasswordNavLink.tsx
@@ -38,7 +38,7 @@ class ChangePasswordNavLink extends React.Component<Props> {
    if (!this.hasPermissionToSetPassword()) {
      return null;
    }
-    return <NavLink to={passwordUrl} label={t("singleUser.menu.setPasswordNavLink")} className={t("singleUser.menu.setPasswordNavLink")}/>;
+    return <NavLink to={passwordUrl} label={t("singleUser.menu.setPasswordNavLink")} testId="user-password-link"/>;
  }

  hasPermissionToSetPassword = () => {
--- a/scm-ui/ui-webapp/src/users/components/navLinks/SetPermissionsNavLink.tsx
+++ b/scm-ui/ui-webapp/src/users/components/navLinks/SetPermissionsNavLink.tsx
@@ -38,7 +38,7 @@ class ChangePermissionNavLink extends React.Component<Props> {
    if (!this.hasPermissionToSetPermission()) {
      return null;
    }
-    return <NavLink to={permissionsUrl} label={t("singleUser.menu.setPermissionsNavLink")} className={t("singleUser.menu.setPermissionsNavLink")}/>;
+    return <NavLink to={permissionsUrl} label={t("singleUser.menu.setPermissionsNavLink")} testId="user-permissions-link"/>;
  }

  hasPermissionToSetPermission = () => {
--- a/scm-ui/ui-webapp/src/users/components/table/Details.tsx
+++ b/scm-ui/ui-webapp/src/users/components/table/Details.tsx
@@ -24,7 +24,13 @@
 import React from "react";
 import { WithTranslation, withTranslation } from "react-i18next";
 import { User } from "@scm-manager/ui-types";
-import { Checkbox, DateFromNow, MailLink } from "@scm-manager/ui-components";
+import {
+  Checkbox,
+  DateFromNow,
+  MailLink,
+  createAttributesForTesting,
+  replaceSpacesInTestId
+} from "@scm-manager/ui-components";

 type Props = WithTranslation & {
  user: User;
@@ -38,11 +44,11 @@ class Details extends React.Component<Props> {
        <tbody>
          <tr>
            <th>{t("user.name")}</th>
-            <td>{user.name}</td>
+            <td {...createAttributesForTesting(replaceSpacesInTestId(user.name))}>{user.name}</td>
          </tr>
          <tr>
            <th>{t("user.displayName")}</th>
-            <td>{user.displayName}</td>
+            <td {...createAttributesForTesting(replaceSpacesInTestId(user.displayName))}>{user.displayName}</td>
          </tr>
          <tr>
            <th>{t("user.mail")}</th>
--- a/scm-ui/ui-webapp/src/users/components/table/UserRow.tsx
+++ b/scm-ui/ui-webapp/src/users/components/table/UserRow.tsx
@@ -25,7 +25,7 @@ import React from "react";
 import { WithTranslation, withTranslation } from "react-i18next";
 import { Link } from "react-router-dom";
 import { User } from "@scm-manager/ui-types";
-import { Icon } from "@scm-manager/ui-components";
+import { Icon, createAttributesForTesting } from "@scm-manager/ui-components";

 type Props = WithTranslation & {
  user: User;
@@ -33,7 +33,11 @@ type Props = WithTranslation & {

 class UserRow extends React.Component<Props> {
  renderLink(to: string, label: string) {
-    return <Link to={to}>{label}</Link>;
+    return (
+      <Link to={to} {...createAttributesForTesting(label)}>
+        {label}
+      </Link>
+    );
  }

  render() {
--- a/scm-ui/ui-webapp/src/users/containers/SingleUser.tsx
+++ b/scm-ui/ui-webapp/src/users/containers/SingleUser.tsx
@@ -114,13 +114,13 @@ class SingleUser extends React.Component<Props> {
                  icon="fas fa-info-circle"
                  label={t("singleUser.menu.informationNavLink")}
                  title={t("singleUser.menu.informationNavLink")}
-                  className={t("singleUser.menu.informationNavLink")}
+                  testId="user-information-link"
                />
                <SubNavigation
                  to={`${url}/settings/general`}
                  label={t("singleUser.menu.settingsNavLink")}
                  title={t("singleUser.menu.settingsNavLink")}
-                  className={t("singleUser.menu.settingsNavLink")}
+                  testId="user-settings-link"
                >
                  <EditUserNavLink user={user} editUrl={`${url}/settings/general`} />
                  <SetPasswordNavLink user={user} passwordUrl={`${url}/settings/password`} />
--- a/scm-ui/ui-webapp/src/users/containers/Users.tsx
+++ b/scm-ui/ui-webapp/src/users/containers/Users.tsx
@@ -33,8 +33,7 @@ import {
  OverviewPageActions,
  Page,
  PageActions,
-  urls,
-  Loading
+  urls
 } from "@scm-manager/ui-components";
 import { getUsersLink } from "../../modules/indexResource";
 import {
@@ -89,10 +88,6 @@ class Users extends React.Component<Props> {
  render() {
    const { users, loading, error, canAddUsers, t } = this.props;

-    if (loading) {
-      return <Loading />;
-    }
-
    return (
      <Page title={t("users.title")} subtitle={t("users.subtitle")} loading={loading || !users} error={error}>
        {this.renderUserTable()}