From c64e0ad583b3d3bdb7f22eadd2b537ce3f5fdb18 Mon Sep 17 00:00:00 2001 From: Agnieszka Gancarczyk <4890675+agagancarczyk@users.noreply.github.com> Date: Thu, 31 Oct 2024 07:01:26 +0000 Subject: [PATCH] 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 * Improved translations for attributes Signed-off-by: Agnieszka Gancarczyk * improvement Signed-off-by: Agnieszka Gancarczyk * improvement Signed-off-by: Agnieszka Gancarczyk * improvement Signed-off-by: Agnieszka Gancarczyk * improvement Signed-off-by: Agnieszka Gancarczyk * improved fetching translations in NewAttributeSettings Signed-off-by: Agnieszka Gancarczyk * improved fetching translations in NewAttributeSettings Signed-off-by: Agnieszka Gancarczyk * cleanup Signed-off-by: Agnieszka Gancarczyk * cleanup Signed-off-by: Agnieszka Gancarczyk --------- Signed-off-by: Agnieszka Gancarczyk --- .../admin/messages/messages_en.properties | 3 +- .../realm-settings/NewAttributeSettings.tsx | 95 +++++---- .../user-profile/AttributesGroupForm.tsx | 180 ++++++++---------- .../attribute/AddTranslationsDialog.tsx | 7 +- 4 files changed, 133 insertions(+), 152 deletions(-) diff --git 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 index c1495d6bfa..c997bb2083 100644 --- 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 @@ -3280,4 +3280,5 @@ noGroupMemberships=No memberships termsAndConditionsDeclined=You need to accept the Terms and Conditions to continue somethingWentWrong=Something went wrong somethingWentWrongDescription=Sorry, an unexpected error has occurred. -tryAgain=Try again \ No newline at end of file +tryAgain=Try again +errorSavingTranslations=Error saving translations\: '{{error}}' diff --git a/js/apps/admin-ui/src/realm-settings/NewAttributeSettings.tsx b/js/apps/admin-ui/src/realm-settings/NewAttributeSettings.tsx index 84fb384746..7564a5d158 100644 --- 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( - 2, - formData.displayName.length - 1, - ); - const filteredTranslations: Array<{ - locale: string; - value: string; - }> = []; - const allTranslations = Object.entries(translations).map( - ([key, value]) => ({ - key, + const formattedKey = + formData.displayName?.substring( + 2, + formData.displayName.length - 1, + ) || ""; + + const filteredTranslations: TranslationForm[] = Object.entries( + translations, + ) + .filter(([key]) => key === formattedKey) + .map(([_, value]) => ({ + locale: selectedLocale, value, - }), - ); + })); - allTranslations.forEach((translation) => { - if (translation.key === formattedKey) { - filteredTranslations.push({ - locale: selectedLocale, - value: translation.value, - }); - } - }); - - const translationToSave: any = { - key: formattedKey, - translations: filteredTranslations, - }; - - translationsToSave.push(translationToSave); + if (filteredTranslations.length > 0) { + translationsToSave.push({ + key: formattedKey, + translations: filteredTranslations, + }); + } } 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; } diff --git a/js/apps/admin-ui/src/realm-settings/user-profile/AttributesGroupForm.tsx b/js/apps/admin-ui/src/realm-settings/user-profile/AttributesGroupForm.tsx index 220bc22a7e..7642fa6ccb 100644 --- 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): 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(); - const [translationsData, setTranslationsData] = useState({ + + 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,29 +253,33 @@ export default function AttributesGroupForm() { try { if ( - translationsData.displayHeader && + translationsData && translationsData.displayHeader.translations.length > 0 ) { for (const translation of translationsData.displayHeader.translations) { - await addLocalization( - translationsData.displayHeader.key, - translation.locale, - translation.value, - ); + if (translation.locale && translation.value) { + await addLocalization( + translationsData.displayHeader.key, + translation.locale, + translation.value, + ); + } } } if ( - translationsData.displayDescription && + translationsData && translationsData.displayDescription.translations.length > 0 ) { for (const translation of translationsData.displayDescription .translations) { - await addLocalization( - translationsData.displayDescription.key, - translation.locale, - translation.value, - ); + if (translation.locale && translation.value) { + await addLocalization( + translationsData.displayDescription.key, + translation.locale, + translation.value, + ); + } } } } catch (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() !== "", diff --git 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 index 5505be2e3c..a7eae8be8e 100644 --- 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] || "", ); });