fixed flaky-ness and removed deprecated select (#30078)

fixes: #29507

Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>

js/apps/admin-ui/cypress/e2e/realm_settings_user_profile_enabled.spec.ts

@@ -9,7 +9,6 @@ import { keycloakBefore } from "../support/util/keycloak_hooks";
 import ModalUtils from "../support/util/ModalUtils";
 import RealmSettingsPage from "../support/pages/admin-ui/manage/realm_settings/RealmSettingsPage";
 import CreateUserPage from "../support/pages/admin-ui/manage/users/CreateUserPage";
-import { UserProfileConfig } from "@keycloak/keycloak-admin-client/lib/defs/userProfileMetadata";
 
 const loginPage = new LoginPage();
 const sidebarPage = new SidebarPage();
@@ -28,37 +27,18 @@ const getAttributesGroupTab = () => userProfileTab.goToAttributesGroupTab();
 const usernameAttributeName = "username";
 const emailAttributeName = "email";
 
-let defaultUserProfile: UserProfileConfig;
-
 describe("User profile tabs", () => {
   const realmName = "Realm_" + uuid();
   const attributeName = "Test";
   const attributeDisplayName = "Test display name";
 
-  before(() => {
-    cy.wrap(null).then(async () => {
-      await adminClient.createRealm(realmName);
+  before(() => adminClient.createRealm(realmName));
 
-      defaultUserProfile = await adminClient.getUserProfile(realmName);
-    });
-  });
-
-  after(() =>
-    cy.wrap(null).then(async () => await adminClient.deleteRealm(realmName)),
-  );
+  after(() => adminClient.deleteRealm(realmName));
 
   beforeEach(() => {
     loginPage.logIn();
     keycloakBefore();
-    cy.wrap(null).then(async () => {
-      await adminClient.updateUserProfile(realmName, defaultUserProfile);
-
-      await adminClient.updateRealm(realmName, {
-        registrationEmailAsUsername: false,
-        editUsernameAllowed: false,
-      });
-    });
-
     sidebarPage.goToRealm(realmName);
     sidebarPage.goToRealmSettings();
   });
@@ -91,11 +71,12 @@ describe("User profile tabs", () => {
     });
 
     it("Modifies existing attribute and performs save", () => {
+      const attrName = "ModifyTest";
       getUserProfileTab();
-      createAttributeDefinition(attributeName);
+      createAttributeDefinition(attrName);
 
       userProfileTab
-        .selectElementInList(attributeName)
+        .selectElementInList(attrName)
         .editAttribute("Edited display name")
         .saveAttributeCreation()
         .assertNotificationSaved();
@@ -103,7 +84,6 @@ describe("User profile tabs", () => {
 
     it("Adds and removes validator to/from existing attribute and performs save", () => {
       getUserProfileTab();
-      createAttributeDefinition(attributeName);
 
       userProfileTab
         .selectElementInList(attributeName)
@@ -120,19 +100,16 @@ describe("User profile tabs", () => {
   });
 
   describe("Attribute groups sub tab tests", () => {
-    it.skip("Deletes an attributes group", () => {
-      const group = "Test";
-      cy.wrap(null).then(() =>
-        adminClient.patchUserProfile(realmName, {
-          groups: [{ name: group }],
-        }),
-      );
+    const group = "Test" + uuid();
 
+    before(() => adminClient.addGroupToProfile(realmName, group));
+
+    it("Deletes an attributes group", () => {
       getUserProfileTab();
       getAttributesGroupTab();
       listingPage.deleteItem(group);
       modalUtils.confirmModal();
-      listingPage.checkEmptyList();
+      listingPage.itemExist(group, false);
     });
   });
 
@@ -202,6 +179,8 @@ describe("User profile tabs", () => {
         .setAttributeValue(emailAttributeName, `testuser9-${uuid()}@gmail.com`)
         .update()
         .assertNotificationUpdated();
+
+      deleteAttributeDefinition(attrName);
     });
 
     it("Checks that not required attribute with permissions to view/edit is present when user is created", () => {
@@ -216,7 +195,9 @@ describe("User profile tabs", () => {
       realmSettingsPage.goToLoginTab();
       cy.wait(1000);
       realmSettingsPage
+        .setSwitch(realmSettingsPage.emailAsUsernameSwitch, false)
         .assertSwitch(realmSettingsPage.emailAsUsernameSwitch, false)
+        .setSwitch(realmSettingsPage.editUsernameSwitch, false)
         .assertSwitch(realmSettingsPage.editUsernameSwitch, false);
 
       // Create user
@@ -228,6 +209,8 @@ describe("User profile tabs", () => {
         .create()
         .assertNotificationCreated()
         .assertAttributeFieldExists(attrName, true);
+
+      deleteAttributeDefinition(attrName);
     });
 
     it("Checks that required attribute with permissions to view/edit is present and required when user is created", () => {
@@ -251,6 +234,8 @@ describe("User profile tabs", () => {
         .setAttributeValue(attrName, "MyAttribute")
         .create()
         .assertNotificationCreated();
+
+      deleteAttributeDefinition(attrName);
     });
 
     it("Checks that required attribute with permissions to view/edit is accepted when user is created", () => {
@@ -270,6 +255,8 @@ describe("User profile tabs", () => {
         .setAttributeValue(attrName, "MyAttribute")
         .create()
         .assertNotificationCreated();
+
+      deleteAttributeDefinition(attrName);
     });
 
     it("Checks that attribute group is visible when user with existing attribute is created", () => {
@@ -358,6 +345,8 @@ describe("User profile tabs", () => {
         .resetAttributeGroup()
         .saveAttributeCreation()
         .assertNotificationSaved();
+
+      deleteAttributeDefinition(attrName);
     });
 
     it("Checks that attribute with select-annotation is displayed and editable when user is created/edited", () => {
@@ -382,7 +371,7 @@ describe("User profile tabs", () => {
       createUserPage
         .goToCreateUser()
         .assertAttributeLabel(attrName, attrName)
-        .assertAttributeSelect(attrName, supportedOptions, "")
+        .assertAttributeSelect(attrName, supportedOptions, "Choose...")
         .setUsername(userName)
         .setAttributeValueOnSelect(attrName, opt1)
         .create()
@@ -400,6 +389,15 @@ describe("User profile tabs", () => {
     });
   });
 
+  function deleteAttributeDefinition(attrName: string) {
+    sidebarPage.goToRealmSettings();
+    getUserProfileTab();
+    getAttributesTab();
+    listingPage.deleteItem(attrName);
+    modalUtils.confirmModal();
+    masthead.checkNotificationMessage("Attribute deleted");
+  }
+
   function createAttributeDefinition(
     attrName: string,
     attrConfigurer?: (attrConfigurer: UserProfile) => void,

js/apps/admin-ui/cypress/support/pages/admin-ui/manage/users/CreateUserPage.ts

@@ -1,5 +1,6 @@
 import Masthead from "../../Masthead";
 import FormValidation from "../../../../forms/FormValidation";
+import Select from "../../../../forms/Select";
 
 export default class CreateUserPage {
   readonly masthead = new Masthead();
@@ -84,28 +85,11 @@ export default class CreateUserPage {
     expectedOptionsWithoutEmptyOption: string[],
     expectedValue: string,
   ) {
-    this.#getSelectFieldButton(attrName).should(
-      "have.class",
-      "pf-v5-c-select__toggle",
+    Select.assertSelectedItem(
+      this.#getSelectFieldButton(attrName),
+      expectedValue,
     );
 
-    const valueToCheck = expectedValue ? expectedValue : this.emptyOptionValue;
-    this.#getSelectFieldButton(attrName)
-      .find(".pf-v5-c-select__toggle-text")
-      .invoke("text")
-      .should("eq", valueToCheck);
-
-    const expectedOptions = [this.emptyOptionValue].concat(
-      expectedOptionsWithoutEmptyOption,
-    );
-    this.#withSelectExpanded(attrName, () => {
-      this.#getSelectOptions(attrName)
-        .should("have.length", expectedOptions.length)
-        .each(($option, index) =>
-          cy.wrap($option).should("have.text", expectedOptions[index]),
-        );
-    });
-
     return this;
   }
 
@@ -132,33 +116,6 @@ export default class CreateUserPage {
     });
   }
 
-  /**
-   * Runs the given function in the context of an expanded select field.
-   *
-   * This method makes sure that the initial expanded/collapsed state is preserved:
-   * The select field will be expanded before running the function, if it is not already expanded.
-   * It will be collapsed after running the function, when it was not expanded before running the function.
-   *
-   * @param attrName the attribute name of the select field
-   * @param func the function to be applied
-   * @private
-   */
-  #withSelectExpanded(attrName: string, func: () => any) {
-    let wasInitiallyExpanded = false;
-    this.#toggleSelectField(attrName, (currentlyExpanded) => {
-      wasInitiallyExpanded = currentlyExpanded;
-      return !currentlyExpanded;
-    })
-      .then(() => func())
-      .then(() =>
-        // click again on the dropdown to hide the values list, when necessary
-        this.#toggleSelectField(
-          attrName,
-          (currentlyExpanded) => currentlyExpanded && !wasInitiallyExpanded,
-        ),
-      );
-  }
-
   assertAttributeLabel(attrName: string, expectedText: string) {
     cy.get(`.pf-v5-c-form__label[for='${attrName}'] .pf-v5-c-form__label-text`)
       .contains(expectedText)
@@ -193,9 +150,7 @@ export default class CreateUserPage {
   }
 
   setAttributeValueOnSelect(attrName: string, value: string) {
-    this.#withSelectExpanded(attrName, () => {
-      this.#getSelectOptions(attrName).contains(value).click();
-    });
+    Select.selectItem(this.#getSelectFieldButton(attrName), value);
 
     return this;
   }

js/apps/admin-ui/cypress/support/util/AdminClient.ts

@@ -260,14 +260,16 @@ class AdminClient {
     await this.#client.users.updateProfile(merge(userProfile, { realm }));
   }
 
-  async patchUserProfile(realm: string, payload: UserProfileConfig) {
+  async addGroupToProfile(realm: string, groupName: string) {
     await this.#login();
 
     const currentProfile = await this.#client.users.getProfile({ realm });
 
-    await this.#client.users.updateProfile(
-      merge(currentProfile, payload, { realm }),
-    );
+    await this.#client.users.updateProfile({
+      ...currentProfile,
+      realm,
+      ...{ groups: [...currentProfile.groups!, { name: groupName }] },
+    });
   }
 
   async createRealmRole(payload: RoleRepresentation) {

js/apps/admin-ui/src/components/key-value-form/ValueSelect.tsx

@@ -36,7 +36,9 @@ export const ValueSelect = ({
       placeholderText={t("valuePlaceholder")}
     >
       {defaultItem.values.map((item) => (
-        <SelectOption key={item} value={item} />
+        <SelectOption key={item} value={item}>
+          {item}
+        </SelectOption>
       ))}
     </KeycloakSelect>
   ) : (

js/libs/ui-shared/src/user-profile/SelectComponent.tsx

@@ -1,4 +1,11 @@
-import { Select, SelectOption } from "@patternfly/react-core/deprecated";
+import {
+  Chip,
+  ChipGroup,
+  MenuToggle,
+  Select,
+  SelectList,
+  SelectOption,
+} from "@patternfly/react-core";
 import { useState } from "react";
 import { Controller, ControllerRenderProps } from "react-hook-form";
 import {
@@ -7,12 +14,11 @@ import {
   UserProfileFieldProps,
 } from "./UserProfileFields";
 import { UserProfileGroup } from "./UserProfileGroup";
-import { UserFormFields, fieldName, isRequiredAttribute, label } from "./utils";
+import { UserFormFields, fieldName, label } from "./utils";
 
 export const SelectComponent = (props: UserProfileFieldProps) => {
   const { t, form, inputType, attribute } = props;
   const [open, setOpen] = useState(false);
-  const isRequired = isRequiredAttribute(attribute);
   const isMultiValue = inputType === "multiselect";
 
   const setValue = (
@@ -23,7 +29,11 @@ export const SelectComponent = (props: UserProfileFieldProps) => {
       if (field.value.includes(value)) {
         field.onChange(field.value.filter((item: string) => item !== value));
       } else {
-        field.onChange([...field.value, value]);
+        if (Array.isArray(field.value)) {
+          field.onChange([...field.value, value]);
+        } else {
+          field.onChange([value]);
+        }
       }
     } else {
       field.onChange(value);
@@ -46,39 +56,56 @@ export const SelectComponent = (props: UserProfileFieldProps) => {
         control={form.control}
         render={({ field }) => (
           <Select
-            toggleId={attribute.name}
-            onToggle={(_event, b) => setOpen(b)}
+            toggle={(ref) => (
+              <MenuToggle
+                id={attribute.name}
+                ref={ref}
+                onClick={() => setOpen(!open)}
+                isExpanded={open}
+                isFullWidth
+                isDisabled={attribute.readOnly}
+              >
+                {(isMultiValue && Array.isArray(field.value) ? (
+                  <ChipGroup>
+                    {field.value.map((selection, index: number) => (
+                      <Chip
+                        key={index}
+                        onClick={(ev) => {
+                          ev.stopPropagation();
+                          setValue(selection, field);
+                        }}
+                      >
+                        {selection}
+                      </Chip>
+                    ))}
+                  </ChipGroup>
+                ) : (
+                  fetchLabel(field.value)
+                )) || t("choose")}
+              </MenuToggle>
+            )}
             onSelect={(_, value) => {
-              const option = value.toString();
+              const option = value?.toString() || "";
               setValue(option, field);
-              if (!Array.isArray(field.value)) {
+              if (!isMultiValue) {
                 setOpen(false);
               }
             }}
-            selections={
-              field.value ? field.value : isMultiValue ? [] : t("choose")
-            }
-            variant={
-              isMultiValue
-                ? "typeaheadmulti"
-                : options.length >= 10
-                  ? "typeahead"
-                  : "single"
-            }
+            selected={field.value}
             aria-label={t("selectOne")}
             isOpen={open}
-            isDisabled={attribute.readOnly}
-            required={isRequired}
           >
-            {["", ...options].map((option) => (
-              <SelectOption
-                selected={field.value === option}
-                key={option}
-                value={option}
-              >
-                {option ? fetchLabel(option) : t("choose")}
-              </SelectOption>
-            ))}
+            <SelectList>
+              {options.map((option) => (
+                <SelectOption
+                  selected={field.value === option}
+                  key={option}
+                  value={option}
+                >
+                  {fetchLabel(option)}
+                </SelectOption>
+              ))}
+            </SelectList>
           </Select>
         )}
       />

js/libs/ui-shared/src/user-profile/UserProfileFields.tsx

@@ -192,7 +192,8 @@ const FormField = ({
   const inputType = useMemo(() => determineInputType(attribute), [attribute]);
 
   const Component =
-    attribute.multivalued || isMultiValue(value)
+    attribute.multivalued ||
+    (isMultiValue(value) && attribute.annotations?.inputType === undefined)
       ? FIELDS["multi-input"]
       : FIELDS[inputType];