From 4291f4b01bd568f98531dc5754fcc0195c56f222 Mon Sep 17 00:00:00 2001
From: Jon Koops <jonkoops@gmail.com>
Date: Wed, 18 Oct 2023 18:24:30 +0200
Subject: [PATCH] Enhance user profile attribute form UX (#24083)

Enhances the user experience for the create and edit form for User Profile attribute by making the following changes:

- Prevents the scopes from being persisted if an attribute can always be shown.
- Hides the scopes if 'Always' is selected, as the control is not interactive.
- Sets the attribute to be editable by admins by default.

Closes #24081
Closes #23790

Co-authored-by: Pedro Igor <pigor.craveiro@gmail.com>
---
 .../realm-settings/NewAttributeSettings.tsx   |  72 +++--
 .../attribute/AttributeGeneralSettings.tsx    | 260 ++++++++----------
 2 files changed, 159 insertions(+), 173 deletions(-)

diff --git a/js/apps/admin-ui/src/realm-settings/NewAttributeSettings.tsx b/js/apps/admin-ui/src/realm-settings/NewAttributeSettings.tsx
index 7d1bfef6fa..4c5937125c 100644
--- a/js/apps/admin-ui/src/realm-settings/NewAttributeSettings.tsx
+++ b/js/apps/admin-ui/src/realm-settings/NewAttributeSettings.tsx
@@ -42,7 +42,7 @@ export type IndexedValidations = {
   value?: Record<string, unknown>;
 };
 
-type UserProfileAttributeType = Omit<
+type UserProfileAttributeFormFields = Omit<
   UserProfileAttribute,
   "validations" | "annotations"
 > &
@@ -50,6 +50,8 @@ type UserProfileAttributeType = Omit<
   Permission & {
     validations: IndexedValidations[];
     annotations: IndexedAnnotations[];
+    hasSelector: boolean;
+    hasRequiredScopes: boolean;
   };
 
 type Attribute = {
@@ -123,7 +125,7 @@ const CreateAttributeFormContent = ({
 
 export default function NewAttributeSettings() {
   const { realm, attributeName } = useParams<AttributeParams>();
-  const form = useForm<UserProfileAttributeType>();
+  const form = useForm<UserProfileAttributeFormFields>();
   const { t } = useTranslation();
   const navigate = useNavigate();
   const { addAlert, addError } = useAlerts();
@@ -141,11 +143,17 @@ export default function NewAttributeSettings() {
         selector,
         required,
         ...values
-      } =
-        config.attributes!.find(
-          (attribute) => attribute.name === attributeName,
-        ) || {};
-      convertToFormValues(values, form.setValue);
+      } = config.attributes!.find(
+        (attribute) => attribute.name === attributeName,
+      ) || { permissions: { edit: ["admin"] } };
+      convertToFormValues(
+        {
+          ...values,
+          hasSelector: typeof selector !== "undefined",
+          hasRequiredScopes: typeof required?.scopes !== "undefined",
+        },
+        form.setValue,
+      );
       Object.entries(
         flatten<any, any>({ permissions, selector, required }, { safe: true }),
       ).map(([key, value]) => form.setValue(key as any, value));
@@ -168,8 +176,20 @@ export default function NewAttributeSettings() {
     [],
   );
 
-  const save = async (profileConfig: UserProfileAttributeType) => {
-    const validations = profileConfig.validations.reduce(
+  const save = async ({
+    hasSelector,
+    hasRequiredScopes,
+    ...formFields
+  }: UserProfileAttributeFormFields) => {
+    if (!hasSelector) {
+      delete formFields.selector;
+    }
+
+    if (!hasRequiredScopes) {
+      delete formFields.required?.scopes;
+    }
+
+    const validations = formFields.validations.reduce(
       (prevValidations, currentValidations) => {
         prevValidations[currentValidations.key] =
           currentValidations.value || {};
@@ -178,7 +198,7 @@ export default function NewAttributeSettings() {
       {} as Record<string, unknown>,
     );
 
-    const annotations = profileConfig.annotations.reduce(
+    const annotations = formFields.annotations.reduce(
       (obj, item) => Object.assign(obj, { [item.key]: item.value }),
       {},
     );
@@ -194,18 +214,14 @@ export default function NewAttributeSettings() {
           {
             ...attribute,
             name: attributeName,
-            displayName: profileConfig.displayName!,
-            selector: profileConfig.selector,
-            permissions: profileConfig.permissions!,
+            displayName: formFields.displayName!,
+            selector: formFields.selector,
+            permissions: formFields.permissions!,
             annotations,
             validations,
           },
-          profileConfig.isRequired
-            ? { required: profileConfig.required }
-            : undefined,
-          profileConfig.group
-            ? { group: profileConfig.group }
-            : { group: null },
+          formFields.isRequired ? { required: formFields.required } : undefined,
+          formFields.group ? { group: formFields.group } : { group: null },
         );
       });
 
@@ -213,20 +229,16 @@ export default function NewAttributeSettings() {
       config?.attributes!.concat([
         Object.assign(
           {
-            name: profileConfig.name,
-            displayName: profileConfig.displayName!,
-            required: profileConfig.isRequired
-              ? profileConfig.required
-              : undefined,
-            selector: profileConfig.selector,
-            permissions: profileConfig.permissions!,
+            name: formFields.name,
+            displayName: formFields.displayName!,
+            required: formFields.isRequired ? formFields.required : undefined,
+            selector: formFields.selector,
+            permissions: formFields.permissions!,
             annotations,
             validations,
           },
-          profileConfig.isRequired
-            ? { required: profileConfig.required }
-            : undefined,
-          profileConfig.group ? { group: profileConfig.group } : undefined,
+          formFields.isRequired ? { required: formFields.required } : undefined,
+          formFields.group ? { group: formFields.group } : undefined,
         ),
       ] as UserProfileAttribute);
 
diff --git a/js/apps/admin-ui/src/realm-settings/user-profile/attribute/AttributeGeneralSettings.tsx b/js/apps/admin-ui/src/realm-settings/user-profile/attribute/AttributeGeneralSettings.tsx
index 3df5c78765..d324e55601 100644
--- a/js/apps/admin-ui/src/realm-settings/user-profile/attribute/AttributeGeneralSettings.tsx
+++ b/js/apps/admin-ui/src/realm-settings/user-profile/attribute/AttributeGeneralSettings.tsx
@@ -45,16 +45,14 @@ export const AttributeGeneralSettings = () => {
   const { attributeName } = useParams<AttributeParams>();
   const editMode = attributeName ? true : false;
 
-  const selectedScopes = useWatch({
+  const hasSelector = useWatch({
     control: form.control,
-    name: "selector.scopes",
-    defaultValue: [],
+    name: "hasSelector",
   });
 
-  const requiredScopes = useWatch({
+  const hasRequiredScopes = useWatch({
     control: form.control,
-    name: "required.scopes",
-    defaultValue: [],
+    name: "hasRequiredScopes",
   });
 
   const required = useWatch({
@@ -69,6 +67,15 @@ export const AttributeGeneralSettings = () => {
   if (!clientScopes) {
     return <KeycloakSpinner />;
   }
+
+  function setHasSelector(hasSelector: boolean) {
+    form.setValue("hasSelector", hasSelector);
+  }
+
+  function setHasRequiredScopes(hasRequiredScopes: boolean) {
+    form.setValue("hasRequiredScopes", hasRequiredScopes);
+  }
+
   return (
     <FormAccess role="manage-realm" isHorizontal>
       <FormGroup
@@ -164,87 +171,71 @@ export const AttributeGeneralSettings = () => {
             <Radio
               id="always"
               data-testid="always"
-              isChecked={selectedScopes.length === clientScopes.length}
+              isChecked={!hasSelector}
               name="enabledWhen"
               label={t("always")}
-              onChange={(value) => {
-                if (value) {
-                  form.setValue(
-                    "selector.scopes",
-                    clientScopes.map((s) => s.name),
-                  );
-                } else {
-                  form.setValue("selector.scopes", []);
-                }
-              }}
+              onChange={() => setHasSelector(false)}
               className="pf-u-mb-md"
             />
             <Radio
               id="scopesAsRequested"
               data-testid="scopesAsRequested"
-              isChecked={selectedScopes.length !== clientScopes.length}
+              isChecked={hasSelector}
               name="enabledWhen"
               label={t("scopesAsRequested")}
-              onChange={(value) => {
-                if (value) {
-                  form.setValue("selector.scopes", []);
-                } else {
-                  form.setValue(
-                    "selector.scopes",
-                    clientScopes.map((s) => s.name),
-                  );
-                }
-              }}
+              onChange={() => setHasSelector(true)}
               className="pf-u-mb-md"
             />
           </FormGroup>
-          <FormGroup fieldId="kc-scope-enabled-when">
-            <Controller
-              name="selector.scopes"
-              control={form.control}
-              defaultValue={clientScopes.map((s) => s.name)}
-              render={({ field }) => (
-                <Select
-                  name="scopes"
-                  data-testid="enabled-when-scope-field"
-                  variant={SelectVariant.typeaheadMulti}
-                  typeAheadAriaLabel="Select"
-                  chipGroupProps={{
-                    numChips: 3,
-                    expandedText: t("hide"),
-                    collapsedText: t("showRemaining"),
-                  }}
-                  onToggle={(isOpen) => setSelectEnabledWhenOpen(isOpen)}
-                  selections={field.value}
-                  onSelect={(_, selectedValue) => {
-                    const option = selectedValue.toString();
-                    let changedValue = [""];
-                    if (field.value) {
-                      changedValue = field.value.includes(option)
-                        ? field.value.filter((item: string) => item !== option)
-                        : [...field.value, option];
-                    } else {
-                      changedValue = [option];
-                    }
-
-                    field.onChange(changedValue);
-                  }}
-                  onClear={(selectedValues) => {
-                    selectedValues.stopPropagation();
-                    field.onChange([]);
-                  }}
-                  isOpen={selectEnabledWhenOpen}
-                  isDisabled={selectedScopes.length === clientScopes.length}
-                  aria-labelledby={"scope"}
-                >
-                  {clientScopes.map((option) => (
-                    <SelectOption key={option.name} value={option.name} />
-                  ))}
-                </Select>
-              )}
-            />
-          </FormGroup>
+          {hasSelector && (
+            <FormGroup fieldId="kc-scope-enabled-when">
+              <Controller
+                name="selector.scopes"
+                control={form.control}
+                defaultValue={[]}
+                render={({ field }) => (
+                  <Select
+                    name="scopes"
+                    data-testid="enabled-when-scope-field"
+                    variant={SelectVariant.typeaheadMulti}
+                    typeAheadAriaLabel="Select"
+                    chipGroupProps={{
+                      numChips: 3,
+                      expandedText: t("hide"),
+                      collapsedText: t("showRemaining"),
+                    }}
+                    onToggle={(isOpen) => setSelectEnabledWhenOpen(isOpen)}
+                    selections={field.value}
+                    onSelect={(_, selectedValue) => {
+                      const option = selectedValue.toString();
+                      let changedValue = [""];
+                      if (field.value) {
+                        changedValue = field.value.includes(option)
+                          ? field.value.filter(
+                              (item: string) => item !== option,
+                            )
+                          : [...field.value, option];
+                      } else {
+                        changedValue = [option];
+                      }
 
+                      field.onChange(changedValue);
+                    }}
+                    onClear={(selectedValues) => {
+                      selectedValues.stopPropagation();
+                      field.onChange([]);
+                    }}
+                    isOpen={selectEnabledWhenOpen}
+                    aria-labelledby={"scope"}
+                  >
+                    {clientScopes.map((option) => (
+                      <SelectOption key={option.name} value={option.name} />
+                    ))}
+                  </Select>
+                )}
+              />
+            </FormGroup>
+          )}
           <Divider />
           <FormGroup
             label={t("required")}
@@ -311,87 +302,70 @@ export const AttributeGeneralSettings = () => {
                 <Radio
                   id="requiredAlways"
                   data-testid="requiredAlways"
-                  isChecked={requiredScopes.length === clientScopes.length}
+                  isChecked={!hasRequiredScopes}
                   name="requiredWhen"
                   label={t("always")}
-                  onChange={(value) => {
-                    if (value) {
-                      form.setValue(
-                        "required.scopes",
-                        clientScopes.map((s) => s.name),
-                      );
-                    } else {
-                      form.setValue("required.scopes", []);
-                    }
-                  }}
+                  onChange={() => setHasRequiredScopes(false)}
                   className="pf-u-mb-md"
                 />
                 <Radio
                   id="requiredScopesAsRequested"
                   data-testid="requiredScopesAsRequested"
-                  isChecked={requiredScopes.length !== clientScopes.length}
+                  isChecked={hasRequiredScopes}
                   name="requiredWhen"
                   label={t("scopesAsRequested")}
-                  onChange={(value) => {
-                    if (value) {
-                      form.setValue("required.scopes", []);
-                    } else {
-                      form.setValue(
-                        "required.scopes",
-                        clientScopes.map((s) => s.name),
-                      );
-                    }
-                  }}
+                  onChange={() => setHasRequiredScopes(true)}
                   className="pf-u-mb-md"
                 />
               </FormGroup>
-              <FormGroup fieldId="kc-scope-required-when">
-                <Controller
-                  name="required.scopes"
-                  control={form.control}
-                  defaultValue={[]}
-                  render={({ field }) => (
-                    <Select
-                      name="scopeRequired"
-                      data-testid="required-when-scope-field"
-                      variant={SelectVariant.typeaheadMulti}
-                      typeAheadAriaLabel="Select"
-                      chipGroupProps={{
-                        numChips: 3,
-                        expandedText: t("hide"),
-                        collapsedText: t("showRemaining"),
-                      }}
-                      onToggle={(isOpen) => setSelectRequiredForOpen(isOpen)}
-                      selections={field.value}
-                      onSelect={(_, selectedValue) => {
-                        const option = selectedValue.toString();
-                        let changedValue = [""];
-                        if (field.value) {
-                          changedValue = field.value.includes(option)
-                            ? field.value.filter(
-                                (item: string) => item !== option,
-                              )
-                            : [...field.value, option];
-                        } else {
-                          changedValue = [option];
-                        }
-                        field.onChange(changedValue);
-                      }}
-                      onClear={(selectedValues) => {
-                        selectedValues.stopPropagation();
-                        field.onChange([]);
-                      }}
-                      isOpen={selectRequiredForOpen}
-                      isDisabled={requiredScopes.length === clientScopes.length}
-                      aria-labelledby={"scope"}
-                    >
-                      {clientScopes.map((option) => (
-                        <SelectOption key={option.name} value={option.name} />
-                      ))}
-                    </Select>
-                  )}
-                />
-              </FormGroup>
+              {hasRequiredScopes && (
+                <FormGroup fieldId="kc-scope-required-when">
+                  <Controller
+                    name="required.scopes"
+                    control={form.control}
+                    defaultValue={[]}
+                    render={({ field }) => (
+                      <Select
+                        name="scopeRequired"
+                        data-testid="required-when-scope-field"
+                        variant={SelectVariant.typeaheadMulti}
+                        typeAheadAriaLabel="Select"
+                        chipGroupProps={{
+                          numChips: 3,
+                          expandedText: t("hide"),
+                          collapsedText: t("showRemaining"),
+                        }}
+                        onToggle={(isOpen) => setSelectRequiredForOpen(isOpen)}
+                        selections={field.value}
+                        onSelect={(_, selectedValue) => {
+                          const option = selectedValue.toString();
+                          let changedValue = [""];
+                          if (field.value) {
+                            changedValue = field.value.includes(option)
+                              ? field.value.filter(
+                                  (item: string) => item !== option,
+                                )
+                              : [...field.value, option];
+                          } else {
+                            changedValue = [option];
+                          }
+                          field.onChange(changedValue);
+                        }}
+                        onClear={(selectedValues) => {
+                          selectedValues.stopPropagation();
+                          field.onChange([]);
+                        }}
+                        isOpen={selectRequiredForOpen}
+                        aria-labelledby={"scope"}
+                      >
+                        {clientScopes.map((option) => (
+                          <SelectOption key={option.name} value={option.name} />
+                        ))}
+                      </Select>
+                    )}
+                  />
+                </FormGroup>
+              )}
             </>
           )}
         </>