Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metadata Editor / support multilingual values in certain fields #1011

Merged
merged 8 commits into from
Oct 9, 2024

Conversation

jahow
Copy link
Collaborator

@jahow jahow commented Oct 4, 2024

Description

This PR introduces support for reading and writing multilingual values in various fields of a catalog record:

These fields are currently:

  • title and abstract of a record
  • name of an organization
  • name and description on an online resource
  • description of a geographical extent
  • text of a constraint or license
  • label of a keyword

The converters (both ISO and DCAT-AP) are now able to read and write translations alongside a field value.

Note: in the model, a record has a mandatory default language. Multilingual fields have a default value (in the default language). They can also have values in other languages, these values are called translations.

A record also have a field called otherLanguages which lists all languages in which translations have been found.

The Metadata Editor currently only modifies the default values of multilingual fields; this means that translations should not be impacted at all by the app for now.

This PR has the important effect that multilingual fields will not lose their translations if modified in the editor; the translations can not be edited but at least they are preserved.

Architectural changes

none

Quality Assurance Checklist

  • Commit history is devoid of any merge commits and readable to facilitate reviews
  • If new logic ⚙️ is introduced: unit tests were added
  • If new user stories 🤏 are introduced: E2E tests were added
  • If new UI components 🕹️ are introduced: corresponding stories in Storybook were created
  • If breaking changes 🪚 are introduced: add the breaking change label
  • If bugs 🐞 are fixed: add the backport <release branch> label
  • The documentation website 📚 has received the love it deserves

Copy link
Contributor

github-actions bot commented Oct 4, 2024

Affected libs: api-metadata-converter, api-repository, feature-catalog, feature-record, feature-router, feature-editor, feature-search, feature-map, feature-dataviz, feature-auth, ui-search, common-domain, common-fixtures, ui-elements, feature-notifications, ui-catalog, util-shared, ui-widgets, ui-inputs, ui-layout, ui-map, ui-dataviz, util-i18n, util-app-config,
Affected apps: datahub, metadata-converter, metadata-editor, demo, webcomponents, search, map-viewer, datafeeder, data-platform,

  • 🚀 Build and deploy storybook and demo on GitHub Pages
  • 📦 Build and push affected docker images

Copy link
Contributor

github-actions bot commented Oct 4, 2024

📷 Screenshots are here!

@jahow jahow force-pushed the converter-support-multilingual-translations-per-field branch 2 times, most recently from c03413e to 44a2a7f Compare October 4, 2024 19:31
@coveralls
Copy link

coveralls commented Oct 4, 2024

Coverage Status

coverage: 83.434% (-0.2%) from 83.65%
when pulling e42e934 on converter-support-multilingual-translations-per-field
into 8237040 on main.

@jahow jahow force-pushed the converter-support-multilingual-translations-per-field branch from 44a2a7f to de3cc97 Compare October 4, 2024 20:58
@@ -100,6 +98,9 @@ export class RecordFormComponent implements AfterViewInit {
keywords: hasPrevious ? this.record.keywords : [],
topics: hasPrevious ? this.record.topics : [],
onlineResources: [],
otherLanguages: [],
defaultLanguage: 'en',
overviews: [],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have adjusted these fixtures to make them correct; there were typing errors that were hidden because of the cast to CatalogRecord below

@jahow jahow force-pushed the converter-support-multilingual-translations-per-field branch from de3cc97 to 829b65f Compare October 4, 2024 21:03
defaultLanguage: LanguageCode
): string {
const dataset = getDatasetNode(dataStore, recordNode)
const [title] = readLocalizedValue<RecordTranslations>(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

readLocalizedValue is introduced in this PR and does two things:

  • add any found translations to the translations object given to it
  • return both the default value and the translations array as a tuple: [value, translations]

@jahow
Copy link
Collaborator Author

jahow commented Oct 7, 2024

I took the opportunity to better document the datahub & editor apps!

Copy link
Collaborator

@tkohr tkohr left a comment

Choose a reason for hiding this comment

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

Thanks, @jahow, for the huge work and for reworking the doc! Not sure if I got everything, but as far as I can see PR LGTM.

apps/datahub/README.md Show resolved Hide resolved
@jahow
Copy link
Collaborator Author

jahow commented Oct 9, 2024

@tkohr thank you for the review!!

@jahow jahow merged commit 822d2a4 into main Oct 9, 2024
13 checks passed
@jahow jahow deleted the converter-support-multilingual-translations-per-field branch October 9, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants