Fixed persisting translations for attribute groups and improved errors for empty translations on attribute/attribute groups save (#33943)

* added fix for attribute groups Signed-off-by: Agnieszka Gancarczyk <agagancarczyk@gmail.com> * Improved translations for attributes Signed-off-by: Agnieszka Gancarczyk <agagancarczyk@gmail.com> * improvement Signed-off-by: Agnieszka Gancarczyk <agagancarczyk@gmail.com> * improvement Signed-off-by: Agnieszka Gancarczyk <agagancarczyk@gmail.com> * improvement Signed-off-by: Agnieszka Gancarczyk <agagancarczyk@gmail.com> * improvement Signed-off-by: Agnieszka Gancarczyk <agagancarczyk@gmail.com> * improved fetching translations in NewAttributeSettings Signed-off-by: Agnieszka Gancarczyk <agagancarczyk@gmail.com> * improved fetching translations in NewAttributeSettings Signed-off-by: Agnieszka Gancarczyk <agagancarczyk@gmail.com> * cleanup Signed-off-by: Agnieszka Gancarczyk <agagancarczyk@gmail.com> * cleanup Signed-off-by: Agnieszka Gancarczyk <agagancarczyk@gmail.com> --------- Signed-off-by: Agnieszka Gancarczyk <agagancarczyk@gmail.com>
2024-10-31 07:01:26 +00:00 · 2024-10-31 07:01:26 +00:00 · c64e0ad583
commit c64e0ad583
parent abb7c414ab
4 changed files with 133 additions and 152 deletions
--- a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties
+++ b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties
@ -3281,3 +3281,4 @@ termsAndConditionsDeclined=You need to accept the Terms and Conditions to contin
 somethingWentWrong=Something went wrong
 somethingWentWrongDescription=Sorry, an unexpected error has occurred.
 tryAgain=Try again
+errorSavingTranslations=Error saving translations\: '{{error}}'
--- a/js/apps/admin-ui/src/realm-settings/NewAttributeSettings.tsx
+++ b/js/apps/admin-ui/src/realm-settings/NewAttributeSettings.tsx
@ -1,3 +1,4 @@
+/* eslint-disable @typescript-eslint/no-unused-vars */
 import type {
  UserProfileAttribute,
  UserProfileConfig,
@ -61,6 +62,7 @@ type UserProfileAttributeFormFields = Omit<
    annotations: IndexedAnnotations[];
    hasSelector: boolean;
    hasRequiredScopes: boolean;
+    translations?: TranslationForm[];
  };

 type Attribute = {
@ -172,7 +174,8 @@ export default function NewAttributeSettings() {

  useFetch(
    async () => {
-      const translationsToSave: any[] = [];
+      const translationsToSave: Translations[] = [];
+
      await Promise.all(
        combinedLocales.map(async (selectedLocale) => {
          try {
@ -183,55 +186,50 @@ export default function NewAttributeSettings() {
              });

            const formData = form.getValues();
-            const formattedKey = formData.displayName?.substring(
+            const formattedKey =
+              formData.displayName?.substring(
                2,
                formData.displayName.length - 1,
-            );
-            const filteredTranslations: Array<{
-              locale: string;
-              value: string;
-            }> = [];
-            const allTranslations = Object.entries(translations).map(
-              ([key, value]) => ({
-                key,
-                value,
-              }),
-            );
+              ) || "";

-            allTranslations.forEach((translation) => {
-              if (translation.key === formattedKey) {
-                filteredTranslations.push({
+            const filteredTranslations: TranslationForm[] = Object.entries(
+              translations,
+            )
+              .filter(([key]) => key === formattedKey)
+              .map(([_, value]) => ({
                locale: selectedLocale,
-                  value: translation.value,
-                });
-              }
-            });
+                value,
+              }));

-            const translationToSave: any = {
+            if (filteredTranslations.length > 0) {
+              translationsToSave.push({
                key: formattedKey,
                translations: filteredTranslations,
-            };
-
-            translationsToSave.push(translationToSave);
+              });
+            }
          } catch (error) {
-            console.error(
-              `Error fetching translations for ${selectedLocale}:`,
-              error,
-            );
+            addError("errorSavingTranslations", error);
          }
        }),
      );
+
      return translationsToSave;
    },
-    (translationsToSaveData) => {
-      setTranslationsData(() => ({
-        key: translationsToSaveData[0].key,
-        translations: translationsToSaveData.flatMap(
-          (translationData) => translationData.translations,
-        ),
-      }));
+    (translationsToSave) => {
+      if (translationsToSave && translationsToSave.length > 0) {
+        const allTranslations = translationsToSave.flatMap(
+          (translation) => translation.translations,
+        );
+
+        setTranslationsData({
+          key: translationsToSave[0].key,
+          translations: allTranslations,
+        });
+
+        form.setValue("translations", allTranslations);
+      }
    },
-    [combinedLocales],
+    [combinedLocales, realmName, form],
  );

  useFetch(
@ -282,8 +280,9 @@ export default function NewAttributeSettings() {

  const saveTranslations = async () => {
    try {
-      const nonEmptyTranslations = translationsData.translations.map(
-        async (translation) => {
+      const nonEmptyTranslations = translationsData.translations
+        .filter((translation) => translation.value.trim() !== "")
+        .map(async (translation) => {
          try {
            await adminClient.realms.addLocalization(
              {
@ -293,11 +292,11 @@ export default function NewAttributeSettings() {
              },
              translation.value,
            );
-          } catch {
-            console.error(`Error saving translation for ${translation.locale}`);
+          } catch (error) {
+            addError(t("errorSavingTranslations"), error);
          }
-        },
-      );
+        });
+
      await Promise.all(nonEmptyTranslations);
    } catch (error) {
      console.error(`Error saving translations: ${error}`);
@ -377,7 +376,7 @@ export default function NewAttributeSettings() {
        (translation) => translation.value.trim() !== "",
      );

-      if (!hasNonEmptyTranslations && !formFields.displayName) {
+      if (!hasNonEmptyTranslations) {
        addError("createAttributeError", t("translationError"));
        return;
      }
--- a/js/apps/admin-ui/src/realm-settings/user-profile/AttributesGroupForm.tsx
+++ b/js/apps/admin-ui/src/realm-settings/user-profile/AttributesGroupForm.tsx
@ -1,3 +1,4 @@
+/* eslint-disable @typescript-eslint/no-empty-function */
 import type { UserProfileGroup } from "@keycloak/keycloak-admin-client/lib/defs/userProfileMetadata";
 import {
  HelpItem,
@ -17,7 +18,6 @@ import {
  TextContent,
  TextInput,
 } from "@patternfly/react-core";
-import { GlobeRouteIcon } from "@patternfly/react-icons";
 import { useEffect, useMemo, useState } from "react";
 import {
  FormProvider,
@ -27,7 +27,6 @@ import {
 } from "react-hook-form";
 import { useTranslation } from "react-i18next";
 import { Link, useNavigate, useParams } from "react-router-dom";
-import { useAdminClient } from "../../admin-client";
 import { FormAccess } from "../../components/form/FormAccess";
 import { KeyValueInput } from "../../components/key-value-form/KeyValueInput";
 import type { KeyValueType } from "../../components/key-value-form/key-value-convert";
@ -44,6 +43,8 @@ import {
  AddTranslationsDialog,
  TranslationsType,
 } from "./attribute/AddTranslationsDialog";
+import { GlobeRouteIcon } from "@patternfly/react-icons";
+import { useAdminClient } from "../../admin-client";

 function parseAnnotations(input: Record<string, unknown>): KeyValueType[] {
  return Object.entries(input).reduce((p, [key, value]) => {
@ -77,11 +78,6 @@ type Translations = {
  translations: TranslationForm[];
 };

-type TranslationsSets = {
-  displayHeader: Translations;
-  displayDescription: Translations;
-};
-
 const defaultValues: FormFields = {
  annotations: [],
  displayDescription: "",
@ -112,20 +108,21 @@ export default function AttributesGroupForm() {
  const [addTranslationsModalOpen, toggleModal] = useToggle();
  const regexPattern = /\$\{([^}]+)\}/;
  const [type, setType] = useState<TranslationsType>();
-  const [translationsData, setTranslationsData] = useState<TranslationsSets>({
+
+  const [translationsData, setTranslationsData] = useState({
    displayHeader: {
      key: "",
-      translations: [],
+      translations: [] as TranslationForm[],
    },
    displayDescription: {
      key: "",
-      translations: [],
+      translations: [] as TranslationForm[],
    },
  });

  const matchingGroup = useMemo(
    () => config?.groups?.find(({ name }) => name === params.name),
-    [config?.groups],
+    [config?.groups, params.name],
  );

  useEffect(() => {
@ -138,120 +135,98 @@ export default function AttributesGroupForm() {
      : [];

    form.reset({ ...defaultValues, ...matchingGroup, annotations });
-  }, [matchingGroup]);
+  }, [matchingGroup, form]);

  useEffect(() => {
    form.setValue(
      "displayHeader",
-      matchingGroup
-        ? matchingGroup.displayHeader!
-        : generatedAttributesGroupDisplayName,
+      matchingGroup?.displayHeader || generatedAttributesGroupDisplayName || "",
    );
    form.setValue(
      "displayDescription",
-      matchingGroup
-        ? matchingGroup.displayDescription!
-        : generatedAttributesGroupDisplayDescription,
+      matchingGroup?.displayDescription ||
+        generatedAttributesGroupDisplayDescription ||
+        "",
    );
  }, [
    generatedAttributesGroupDisplayName,
    generatedAttributesGroupDisplayDescription,
+    matchingGroup,
+    form,
  ]);

  useFetch(
    async () => {
      const translationsToSaveDisplayHeader: Translations[] = [];
      const translationsToSaveDisplayDescription: Translations[] = [];
-      const formData = form.getValues();

-      const translationsResults = await Promise.all(
-        combinedLocales.map(async (selectedLocale) => {
+      await Promise.all(
+        combinedLocales.map(async (locale: string) => {
          try {
            const translations =
              await adminClient.realms.getRealmLocalizationTexts({
                realm: realmName,
-                selectedLocale,
+                selectedLocale: locale,
              });

-            const formattedDisplayHeaderKey = formData.displayHeader?.substring(
-              2,
-              formData.displayHeader.length - 1,
-            );
-            const formattedDisplayDescriptionKey =
-              formData.displayDescription?.substring(
-                2,
-                formData.displayDescription.length - 1,
-              );
-
-            return {
-              locale: selectedLocale,
-              headerTranslation: translations[formattedDisplayHeaderKey] ?? "",
-              descriptionTranslation:
-                translations[formattedDisplayDescriptionKey] ?? "",
+            const formData = form.getValues();
+            const extractKey = (value: string | undefined) => {
+              const match = value?.match(/\$\{(.*?)\}/);
+              return match ? match[1] : "";
            };
+
+            const displayHeaderKey = extractKey(formData.displayHeader) || "";
+            const displayDescriptionKey =
+              extractKey(formData.displayDescription) || "";
+
+            const headerTranslation = translations[displayHeaderKey] || "";
+            const descriptionTranslation =
+              translations[displayDescriptionKey] || "";
+
+            if (headerTranslation) {
+              translationsToSaveDisplayHeader.push({
+                key: displayHeaderKey,
+                translations: [{ locale, value: headerTranslation }],
+              });
+            }
+
+            if (descriptionTranslation) {
+              translationsToSaveDisplayDescription.push({
+                key: displayDescriptionKey,
+                translations: [{ locale, value: descriptionTranslation }],
+              });
+            }
          } catch (error) {
-            console.error(
-              `Error fetching translations for ${selectedLocale}:`,
-              error,
-            );
-            return null;
+            console.error(`Error fetching translations for ${locale}:`, error);
          }
        }),
      );

-      translationsResults.forEach((translationsResult) => {
-        if (translationsResult) {
-          const { locale, headerTranslation, descriptionTranslation } =
-            translationsResult;
-          translationsToSaveDisplayHeader.push({
-            key: formData.displayHeader?.substring(
-              2,
-              formData.displayHeader.length - 1,
-            ),
-            translations: [
-              {
-                locale,
-                value: headerTranslation,
-              },
-            ],
-          });
-          translationsToSaveDisplayDescription.push({
-            key: formData.displayDescription?.substring(
-              2,
-              formData.displayDescription.length - 1,
-            ),
-            translations: [
-              {
-                locale,
-                value: descriptionTranslation,
-              },
-            ],
-          });
-        }
-      });
-
-      return {
-        translationsToSaveDisplayHeader,
-        translationsToSaveDisplayDescription,
-      };
-    },
-    (data) => {
-      setTranslationsData({
+      const translationsDataNew = {
        displayHeader: {
-          key: data.translationsToSaveDisplayHeader[0].key,
-          translations: data.translationsToSaveDisplayHeader.flatMap(
-            (translationData) => translationData.translations,
+          key:
+            translationsToSaveDisplayHeader.length > 0
+              ? translationsToSaveDisplayHeader[0].key
+              : "",
+          translations: translationsToSaveDisplayHeader.flatMap(
+            (data) => data.translations,
          ),
        },
        displayDescription: {
-          key: data.translationsToSaveDisplayDescription[0].key,
-          translations: data.translationsToSaveDisplayDescription.flatMap(
-            (translationData) => translationData.translations,
+          key:
+            translationsToSaveDisplayDescription.length > 0
+              ? translationsToSaveDisplayDescription[0].key
+              : "",
+          translations: translationsToSaveDisplayDescription.flatMap(
+            (data) => data.translations,
          ),
        },
-      });
+      };
+
+      setTranslationsData(translationsDataNew);
    },
-    [combinedLocales],
+    () => {},
+    [combinedLocales, realmName, form],
  );

  const saveTranslations = async () => {
@ -278,10 +253,11 @@ export default function AttributesGroupForm() {

    try {
      if (
-        translationsData.displayHeader &&
+        translationsData &&
        translationsData.displayHeader.translations.length > 0
      ) {
        for (const translation of translationsData.displayHeader.translations) {
+          if (translation.locale && translation.value) {
            await addLocalization(
              translationsData.displayHeader.key,
              translation.locale,
@ -289,13 +265,15 @@ export default function AttributesGroupForm() {
            );
          }
        }
+      }

      if (
-        translationsData.displayDescription &&
+        translationsData &&
        translationsData.displayDescription.translations.length > 0
      ) {
        for (const translation of translationsData.displayDescription
          .translations) {
+          if (translation.locale && translation.value) {
            await addLocalization(
              translationsData.displayDescription.key,
              translation.locale,
@ -303,6 +281,7 @@ export default function AttributesGroupForm() {
            );
          }
        }
+      }
    } catch (error) {
      console.error(`Error while processing translations: ${error}`);
    }
@ -331,7 +310,6 @@ export default function AttributesGroupForm() {
        translationsData.displayHeader.translations.some(
          (translation) => translation.value.trim() !== "",
        );
-
      const hasNonEmptyDisplayDescriptionTranslations =
        translationsData.displayDescription.translations.some(
          (translation) => translation.value.trim() !== "",
--- a/js/apps/admin-ui/src/realm-settings/user-profile/attribute/AddTranslationsDialog.tsx
+++ b/js/apps/admin-ui/src/realm-settings/user-profile/attribute/AddTranslationsDialog.tsx
@ -142,10 +142,13 @@ export const AddTranslationsDialog = ({
  useEffect(() => {
    combinedLocales.forEach((locale, rowIndex) => {
      setValue(`translations.${rowIndex}.locale`, locale);
+
+      const translationExists =
+        translations.translations[rowIndex] !== undefined;
      setValue(
        `translations.${rowIndex}.value`,
-        translations.translations.length > 0
-          ? translations.translations[rowIndex].value
+        translationExists
+          ? translations.translations[rowIndex]?.value
          : defaultTranslations[locale] || "",
      );
    });