Skip to content

Translatable configurable metadata#1292

Open
tvdijen wants to merge 3 commits intoOpenConext:mainfrom
tvdijen:feature/configurable-metadata-translations
Open

Translatable configurable metadata#1292
tvdijen wants to merge 3 commits intoOpenConext:mainfrom
tvdijen:feature/configurable-metadata-translations

Conversation

@tvdijen
Copy link
Contributor

@tvdijen tvdijen commented Mar 20, 2024

This is a continuation of #1103 where we made the Organization-data in metadata configurable using the translations-files.

However, the result would be like this:

<md:OrganizationDisplayName lang="en">My fictional organization</md:OrganizationDisplayName>
<md:OrganizationDisplayName lang="nl">My fictional organization</md:OrganizationDisplayName>

Instead of:

<md:OrganizationDisplayName lang="en">My fictional organization</md:OrganizationDisplayName>
<md:OrganizationDisplayName lang="nl">Mijn fictieve organisatie</md:OrganizationDisplayName>

Everything was being translated using the default locale. This PR attempts to fix that.

@tvdijen tvdijen force-pushed the feature/configurable-metadata-translations branch 15 times, most recently from 2fba06d to 1caa10e Compare March 20, 2024 13:48
@tvdijen
Copy link
Contributor Author

tvdijen commented Mar 20, 2024

@MKodde Would you please help me fix the last few unit tests? 🙎🏻‍♂️ I'm stuck and this close to going :goberserk:

@tvdijen tvdijen force-pushed the feature/configurable-metadata-translations branch from 1caa10e to e8bf462 Compare June 5, 2024 15:48
@MKodde
Copy link
Member

MKodde commented Aug 27, 2024

@tvdijen I think I fixed the unit tests. You where on the right track, but needed some additional translations for the different configured languages (en/nl/pt). Now 🤞 the other tests pass.

I also took the liberty to squash the commits, as they are really one atomic whole. But the main reason is to steal (co-author) this commit from you ;)

@MKodde MKodde force-pushed the feature/configurable-metadata-translations branch from 51a7a38 to 8c0383e Compare August 27, 2024 08:52
MKodde pushed a commit to tvdijen/OpenConext-engineblock that referenced this pull request Aug 27, 2024
This is a continuation of OpenConext#1103 where we made the Organization-data in metadata
configurable using the translations-files.

However, the result would be like this:

<md:OrganizationDisplayName lang="en">My fictional organization</md:OrganizationDisplayName>
<md:OrganizationDisplayName lang="nl">My fictional organization</md:OrganizationDisplayName>

Instead of:

<md:OrganizationDisplayName lang="en">My fictional organization</md:OrganizationDisplayName>
<md:OrganizationDisplayName lang="nl">Mijn fictieve organisatie</md:OrganizationDisplayName>

Everything was being translated using the default locale. This PR attempts to fix that.

See: OpenConext#1292
See: OpenConext#1103
@MKodde MKodde force-pushed the feature/configurable-metadata-translations branch from 8c0383e to 0bb2a4e Compare August 27, 2024 08:54
@MKodde MKodde requested review from MKodde and thijskh August 27, 2024 09:05
Copy link
Member

@MKodde MKodde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @tvdijen The good news is that the organization display name is now translated, see one last observation in my feedback below

@baszoetekouw baszoetekouw requested review from johanib and kayjoosten and removed request for thijskh March 17, 2026 08:00
@baszoetekouw baszoetekouw added this to the 7.2.0 milestone Mar 17, 2026
@baszoetekouw baszoetekouw moved this from New to Backlog in PHP development Mar 17, 2026
@kayjoosten
Copy link
Contributor

@tvdijen can you rebase the pr? ill test it after and if it works ill approve it :D

@johanib
Copy link
Contributor

johanib commented Mar 17, 2026

@tvdijen can you rebase the pr? ill test it after and if it works ill approve it :D

I'll rebase it if needed. Need to dive in anyway.

@tvdijen
Copy link
Contributor Author

tvdijen commented Mar 17, 2026

I'm sorry, I'm not going over this struggle again after this PR aged like milk for two years.

tvdijen added 2 commits March 18, 2026 09:11
This is a continuation of OpenConext#1103 where we made the Organization-data in metadata
configurable using the translations-files.

However, the result would be like this:

<md:OrganizationDisplayName lang="en">My fictional organization</md:OrganizationDisplayName>
<md:OrganizationDisplayName lang="nl">My fictional organization</md:OrganizationDisplayName>

Instead of:

<md:OrganizationDisplayName lang="en">My fictional organization</md:OrganizationDisplayName>
<md:OrganizationDisplayName lang="nl">Mijn fictieve organisatie</md:OrganizationDisplayName>

Everything was being translated using the default locale. This PR attempts to fix that.

See: OpenConext#1292
See: OpenConext#1103
@kayjoosten kayjoosten self-assigned this Mar 18, 2026
After storing organization translation keys instead of translated values
in EngineBlockConfiguration, contact persons were inadvertently using the
raw key string ('metadata_organization_name') as their GivenName rather
than the actual translated value.

ContactPerson elements in SAML metadata have no locale support, so the
translation is resolved once at construction time using the default locale.
@kayjoosten kayjoosten force-pushed the feature/configurable-metadata-translations branch from bb87c8e to 9a56b01 Compare March 18, 2026 10:08
@johanib johanib moved this from Backlog to In Progress in PHP development Mar 18, 2026
@kayjoosten
Copy link
Contributor

metadata-before-after

visual representation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

5 participants